CLN: some simple code cleanup

[hydratedguy]

i've changed some inconsistencies on some magic methods's parameters and removed a return from a object initialization

[jbrockmendel]

this looks like it should fail in the status quo. does this change fix anything?

[hydratedguy]

Sorry for the delay, @jbrockmendel ! In this case, for example, it doesn't fail because missing is not directly called. Essentially these changes don't fix anything but code readability and correctness.

[datapythonista]

Thanks @hydratedguy, really nice clean up.

Except the return, I think all the other methods should be failing if used. I checked __len__ and __getitem__ and they raised, if used as expected:

>>> class Foo:
...     def __len__(self, n):
...             return 1
... 
>>> foo = Foo()
>>> len(foo)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __len__() missing 1 required positional argument: 'n'
>>> class A:
...     def __getitem__(self):
...             return 1
... 
>>> a = A()
>>> a['b']
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __getitem__() takes 1 positional argument but 2 were given

Do you mind having a look and see if the affected tests are being executed in the CI, if the classes are being used at all... I think we can probably clean up more than fixing the errors.

[hydratedguy]

Thanks for the feedback, @datapythonista! Gonna do some exploring looking for what you mentioned then.

[hydratedguy]

Hey @datapythonista! Sorry for the delay.

So, i didn't really know how to check if the tests were being executed in the CI, but i tried analyzing the classes on each test (how they are being used and for what purpose) and got to the conclusion that they're not wrong, just the parameters of the magic methods that are not entirely correct. I also checked if these cases appears in other places but they dont.

I really want to help, so if you can give me some pointers i really would appreciate it!

[jbrockmendel]

LGTM

[datapythonista]

Sorry for the delay @hydratedguy. In general, there are few ways to check if a test is executed. One would be to check the list of skipped tests in one of the builds. The faster and most reliable could be to add an assert False into the test, push, and see if the CI fails.

I had another look, and I think in this case it's not the tests that are not being used, but the functionality itself. What I'd try is to delete the __len__ and __missing__ methods completely. They don't seem to be called. Maybe there is a reason why they are there, and then your fix will be fine. But if they are not needed, probably worth removing them instead of fixing them.

[datapythonista]

Also, if you can please merge upstream into your branch, the CI should be working now.

[datapythonista]

Thanks for the clean up @hydratedguy

If in a follow up PR you want to try to delete these methods, which may not be needed, that would be great.