Skip to content

BUG: incorrect type for objects whose __len__ fails - #7397

Merged
njsmith merged 1 commit into
numpy:masterfrom
ahaldane:fix_failed_len
Mar 9, 2016
Merged

BUG: incorrect type for objects whose __len__ fails#7397
njsmith merged 1 commit into
numpy:masterfrom
ahaldane:fix_failed_len

Conversation

@ahaldane

@ahaldane ahaldane commented Mar 9, 2016

Copy link
Copy Markdown
Member

Fixes #7393, see discussion there. Caused by a bug introduced in #7266.

This reverts the behavior of the following code be the same as before #7266

class A(object):
    def __init__(self, data):
        self._data = data
    def __getitem__(self, item):
        return type(self)(self._data[item])

d = A([1,2,3])
np.array(d)
# array([A(1), A(2), A(3)], dtype=object)

@ahaldane

ahaldane commented Mar 9, 2016

Copy link
Copy Markdown
Member Author

Don't merge this, I want to investigate a little more.

@ahaldane ahaldane mentioned this pull request Mar 9, 2016
Closed
@ahaldane

ahaldane commented Mar 9, 2016

Copy link
Copy Markdown
Member Author

Actually I think my fix in #7266 was the right idea, but there was another bug lurking, which I've now fixed. I was missing a call to PyErr_Clear().

It seems that if PySequence_Length raises an error, and it is not cleared, further calls to PySequence_Length will fail too even for valid sequences. I don't understand why that is. In the debugger I can check that PySequence_Length returns -1 immediately before the call to PyErr_Clear, but immediately after it gives the right result (3) for the example above.

@charris

charris commented Mar 9, 2016

Copy link
Copy Markdown
Member

Perhaps related to #7399 ?

@charris charris added this to the 1.11.0 release milestone Mar 9, 2016
@njsmith

njsmith commented Mar 9, 2016

Copy link
Copy Markdown
Member

The patch looks solid to me. @mhvk, any chance you can check if this fixes the astropy issue?

@mhvk

mhvk commented Mar 9, 2016

Copy link
Copy Markdown
Contributor

I did a quick test and yes this solves #7393.

(With numpy master, still two failing tests, but those are on the way to being solved: #7312, #5706)

@njsmith

njsmith commented Mar 9, 2016

Copy link
Copy Markdown
Member

Thanks @mhvk!

@njsmith

njsmith commented Mar 9, 2016

Copy link
Copy Markdown
Member

Oh, and thanks @ahaldane too, of course :-)

njsmith added a commit that referenced this pull request Mar 9, 2016
@njsmith
Merge pull request #7397 from ahaldane/fix_failed_len
cb4b28c 
BUG: incorrect type for objects whose __len__ fails
@njsmith
njsmith merged commit cb4b28c into numpy:master Mar 9, 2016
@charris charris mentioned this pull request Mar 10, 2016
Merged
@charris charris removed this from the 1.11.0 release milestone Mar 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

06 - Regression component: numpy._core

Projects

None yet

Milestone

No milestone

Development

See More

Successfully merging this pull request may close these issues.

4 participants

@ahaldane @charris @njsmith @mhvk