Skip to content

np.isfinite on structured arrays returns a NotImplemented type object (Trac #1175) #1773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
thouis opened this issue Oct 19, 2012 · 9 comments
Labels
00 - Bug component: numpy._core Priority: high High priority, also add milestones for urgent issues
Milestone

Comments

@thouis
Copy link
Contributor

thouis commented Oct 19, 2012

Original ticket http://projects.scipy.org/numpy/ticket/1175 on 2009-07-15 by @pierregm, assigned to unknown.

Consider the current code:

>>> a = np.array(zip(np.arange(3)),dtype=[('a',float)])
>>> np.isfinite(a)
NotImplemented

Instead of a NotImplemented type object, shouldn't np.isfinite raise a NotImplementedError exception when called on a structured array ?

@thouis
Copy link
Contributor Author

thouis commented Oct 19, 2012

Attachment added by @charris on 2009-07-16: notimplemented.patch

@thouis
Copy link
Contributor Author

thouis commented Oct 19, 2012

@charris wrote on 2009-07-16

That seems to be this snippet of code at lines 3276-3290 in ufunc_object.c :

    if (errval < 0) {
        for (i = 0; i < self->nargs; i++) {
            PyArray_XDECREF_ERR(mps[i]);
        }
        if (errval == -1)
            return NULL;
        else {
            /*
             * PyErr_SetString(PyExc_TypeError,"");
             * return NULL;
             */
            Py_INCREF(Py_NotImplemented);
            return Py_NotImplemented;
        }
    }

It should be easy to fix if we decide on that. A grep for Py_NotImplemented shows that it is returned in quite a few places. I've attached a patch.

@thouis
Copy link
Contributor Author

thouis commented Oct 19, 2012

@charris wrote on 2009-07-16

However, note that the choice to return a NotImplemented object instead of raising an error seems to have been deliberate. I'd guess that Travis made the choice and we should find out why.

@thouis
Copy link
Contributor Author

thouis commented Oct 19, 2012

@teoliphant wrote on 2009-07-16

Py_NotImplemented is how you tell Python to give the other object involved in a mixed-type operation a go.

An errval of something besides -1 means that there was a problem in performing the coercion or finding an appropriate function to call. Returning Py_NotImplemented allows other use cases with mixed arrays and other Python objects to work.

Raising an error is not appropriate in all those times. If we want to raise an error at just this time, then some re-factoring is needed. I would rather see effort go towards making ufuncs work on structured arrays. This is possible now with the changes that are being made on the date-time branch.

@thouis
Copy link
Contributor Author

thouis commented Oct 19, 2012

@charris wrote on 2009-07-16

OK,

But there aren't other objects involved in unary functions. Shouldn't they raise an error? Why doesn't python raise an error of nothing can be done?

@thouis
Copy link
Contributor Author

thouis commented Oct 19, 2012

@teoliphant wrote on 2009-07-20

Yes, that is true. That's why I said some re-factoring is needed to handle this correctly. Perhaps this should be rolled in with the re-factoring that was done to try and handle re-using temporaries.

For example, the low-level function could return NotImplemented. Then, this should be converted depending on how it is called. Or, a low-level function could take a flag argument to either return NotImplemented or raise not implemented.

@thouis
Copy link
Contributor Author

thouis commented Oct 19, 2012

@cournape wrote on 2009-07-27

It would be nice to solve this for 1.4.0

@thouis
Copy link
Contributor Author

thouis commented Oct 19, 2012

Milestone changed to 1.4.0 by @cournape on 2009-07-27

@thouis
Copy link
Contributor Author

thouis commented Oct 19, 2012

@teoliphant wrote on 2009-12-04

Fixed by checking to be sure that there are 2-inputs and 1-output before returning NotImplemented. Otherwise, a NotImplemented error is raised. Fixed in r7877.

@thouis thouis closed this as completed Oct 19, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
00 - Bug component: numpy._core Priority: high High priority, also add milestones for urgent issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant