BUG: Segfault for classes with deceptive __len__

[ahaldane]

Fixes #7264

[ahaldane]

Hmm, failures in python2 are fixed by removing the code block a few lines above, which has the comment /* Not exactly sure what this is about... */ :) Looking into it...

[ahaldane]

OK, I have to leave this for a little bit.

Here are notes so I remember what I was thinking. The issue is that python2 and python3 give different results for the example in #7264.

The first reason is this block marked /* Not exactly sure what this is about... */, which in python2 causes the C() from the example to count simply as an object type by testing PyInstance_Check. In other words, in python2 the sequence checks are completely skipped, and we drop directly to the object array case. I'm not sure there is an equivalent instancecheck in python3, as "old style" classes/instances were removed.

However, if we remove that block at least we now get a ValueError in both python3 and python2... but they are different ValueErrors. At first python2 and python3 are similar in that they create an array of NPY_DOUBLE (a bit strange itself) in PyArray_GetArrayParamsFromObject but they differ when calculating the number of dimensions of this array.

This is caused by yet another call to PyInstance_Check in python2, here, which also has a warning /* FIXME: XXX -- what is the correct thing to do here? */. In python2 this case is True, so ndim gets set to 0. In python3 this is False, so ndim gets set to 1 further down during sequence checks. That means that, when trying to assign to the array of doubles, they go down different branches here, each giving a different ValueError.

I am tempted to just remove the first block above, and live with the fact that we get different ValueErrors in python2 and python3. However that probably means there is a bug that will turn up when more people convert from python2 to python3.

[ahaldane]

Fixed up, should be ready. Is a release note needed?

I decided to remove the code blocks involving PyInstance_Check (which only exists in python2). It turns out those were the only two uses in all of numpy! Now python2 and python3 have the same behavior.

This does mean a small change in behavior for python2:

class C:
    def __init__(self):
        pass
    def __getitem__(self, ind):
        if ind in [0,1]:
            return ind
        else:
            raise IndexError()
    def __len__(self):
        return 2

np.array([C()])

Old result:

 array([<__main__.C instance at 0x7fe4ce8eb9e0>], dtype=object)

New result:

 array([[0, 1]])

In words, if a class defines both __len__ and __getitem__, it is now considered a sequence, but it wasn't before.

However, as before, if __len__ is not defined but __getitem__ is, you get an object. Apparently some libraries expect that.

[charris]

@anntzer How did you discover the error?

I think a note would be appropriate, put it in 1.11-notes, as we might as well get it fixed and tested. Note that on Python 2.7 the following class also segfaults

In [3]: class C(object):
    def __getitem__(self, i): raise IndexError
    def __len__(self): return 42
   ...:     

[anntzer]

I was trying to figure out what rules numpy uses to decide between creating an object array and trying to iterate over the array and get floats out of that (basically my starting point was #7233), so at some point I basically wrote the same class as you did and saw the segfault.

[charris]

Thanks @ahaldane .

[ahaldane]

Thanks, and as Post Script note: I updated the final version to remove dead code in _use_default_type, and then made sure that we did not lose the detection of 'user-defined' types which the dead code checked for.