8000 BUG __numpy_ufunc__ does not properly deal with NotImplemented · Issue #5074 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

BUG __numpy_ufunc__ does not properly deal with NotImplemented #5074

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.

Sign up for GitHub

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

Jump to bottom
Closed
mhvk opened this issue Sep 15, 2014 · 8 comments
Closed

BUG __numpy_ufunc__ does not properly deal with NotImplemented #5074

mhvk opened this issue Sep 15, 2014 · 8 comments

Comments

@mhvk
Copy link
Contributor
mhvk commented Sep 15, 2014

In continuing to try to get astropy's Quantity to behave with __numpy_ufunc__, I found what probably is another bug. One of our tests checks that an integer can be used as an __index__ to, e.g., multiply a list, as in:

import numpy as np
np.array([2]) * ['a', 'b']

(which yields ['a', 'b', 'a', 'b']).

But if I make a trivial class that implements __numpy_ufunc__, the NotImplemented does not seem to be passed on correctly:

class MyA(np.ndarray):
    def __numpy_ufunc__(self, ufunc, method, i, inputs, **kwargs):
        result = getattr(ufunc, method)(*((input.view(np.ndarray)
                                           if isinstance(input, np.ndarray)
                                           else input) for input in inputs),
                                        **kwargs)
        print("Got result={}".format(result))
        return result

np.array([2]).view(MyA) * ['a', 'b']

yields

Got result=NotImplemented
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-21-b1e259840c66> in <module>()
----> 1 np.array([2]).view(MyA) * ['a', 'b']

TypeError: __numpy_ufunc__ not implemented for this type.

p.s. This may be related to #4815
@pv
Copy link
Member
pv commented Sep 15, 2014

The behavior you are seeing does not appear to be due to
__numpy_ufunc__. NotImplemented comes from legacy behavior of the
ufunc itself:

>>> np.multiply(np.array([2]), ['a', 'b'])
NotImplemented

@pv
Copy link
Member
pv commented Sep 15, 2014

The problem in this case is that a part of the Python binop __mul__ logic is inside the ufunc, and since the special value NotImplemented is reserved for communication with __numpy_ufunc__ logic, it's not possible to emulate the legacy behavior.

gh-4815 does not seem to be directly related to this, as it's about precedence of Python binops vs. ufuncs.

@mhvk
Copy link
Contributor Author
mhvk commented Sep 15, 2014

@pv - I'm afraid your explanation still leaves me a bit confused: why is the NotImplemented returned by my __numpy_ufunc__ not passed on to the python binop mechanism, so list.__rmul__ can have its shot? As you probably can imagine, this is not really for the example I gave above -- which will hardly ever occur in reality, certainly with a Quantity! -- but more out of a worry that this will eventually bite us in an unexpected way. Most likely, it is simply that I do not understand yet how NotImplemented is used in communication with __numpy_ufunc__; could you perhaps point me to the code (or a possible writeup)?

@pv
Copy link
Member
pv commented Sep 15, 2014

The documentation is here:
http://docs.scipy.org/doc/numpy-dev/reference/arrays.classes.html#numpy.class.__numpy_ufunc__
You may also want to read the corresponding NEP under doc/neps in numpy
repository.
__numpy_ufunc__ uses the same resolution policy as Python binops. This
process is completely separate from Python binops. np.multiply(a, b)
is not the same thing as a*b. When ndarray.__mul__ calls
np.multiply it's too late to decide that in fact it should have
returned NotImplemented.
*
The point simply is that effectively,
class ndarray:
def mul(self, other):
# ... some special casing ...
return np.multiply(self, other)
*
The point is that the behavior where np.multiply returning NotImplemented in some special cases is unfortunate legacy behavior --- this should not really have been implemented in this way, but the logic on deciding when this occurs should have been in ndarray.__mul__ and not in the ufunc np.multiply which by itself doesn't really have anything to do with the Python binary operation mechanism (apart from the fact that ndarray internally implements the binop using it).

@mhvk
Copy link
Contributor Author
mhvk commented Sep 15, 2014

The documentation would certainly not lead one to predict the behaviour seen 😄 The clue would seem to be what you mentioned above, that the binop __mul__ logic is inside the ufunc. It remains unclear to me why the ufunc couldn't just pass on NotImplemented if only one argument is an array instance.

Or is the problem perhaps that in this case the list is interpreted as an ndarray?

Hmm, yes, a simple test would seem to confirm that:

import numpy as np
class Foo(object):
    def __rmul__(self, other):
        return other.__index__() * 'Hurrah! '

np.array([2]) * Foo()
# array(['Hurrah! Hurrah! '], dtype=object)

Rather presumptive of ndarray to think I wanted an ndarray class back! Is solving this an option at all, or is there so much legacy based on this that one better not even attempt it?

@mhvk
Copy link
Contributor Author
mhvk commented Sep 15, 2014

Oops, sorry, I now see you sent a much longer explanation. This does clarify it. I guess this really only leaves the question whether there so much legacy based on this that one better not even attempt trying move the binop stuff out of the ufunc.

@pv
Copy link
Member
pv commented Sep 15, 2014

In principle moving the logic around is possible --- it's just somewhat
difficult to pull it out, as when ufuncs return NotImplemented is not
straightforward. Can't remember from the top of my head --- it's
something like "if the result would be an object array and rmul
exists, taking into account some special cases for string and void
dtypes except in some cases if there are object arrays involved".
*
It's difficult to know how much code is out there relying on the current
behavior (which is somewhat dodgy, as illustrated by your example --- I
don't immediately understand how the rmul can produce an object array...)
*
To fully emulate the current behavior, the ndarray subclass would need
to implement mul et al. in addition to numpy_ufunc.
*
The other choice would be to use an unique special value different from
NotImplemented in numpy_ufunc resolution. Not so elegant as the
binop and ufunc resolution mechanisms would no longer the same...

@mhvk mhvk mentioned this issue Sep 18, 2014
Merged
@mhvk mhvk mentioned this issue Oct 23, 2014
Closed
@mhvk mhvk mentioned this issue Apr 5, 2015
Merged
@mhvk
Copy link
Contributor Author
mhvk commented Jun 19, 2015

I'm closing this one, since it is, effectively, a duplicate of #5844.

@mhvk mhvk closed this as completed Jun 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pv @mhvk
0