You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
BUG __numpy_ufunc__ does not properly deal with NotImplemented #5074
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.
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.
@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)?
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).
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:
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?
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.
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...
Activity
pv commentedon Sep 15, 2014
The behavior you are seeing does not appear to be due to
__numpy_ufunc__. NotImplemented comes from legacy behavior of theufunc itself:
pv commentedon 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 valueNotImplementedis 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 commentedon Sep 15, 2014
@pv - I'm afraid your explanation still leaves me a bit confused: why is the
NotImplementedreturned by my__numpy_ufunc__not passed on to the python binop mechanism, solist.__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 aQuantity! -- 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 howNotImplementedis used in communication with__numpy_ufunc__; could you perhaps point me to the code (or a possible writeup)?pv commentedon 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. Thisprocess is completely separate from Python binops.
np.multiply(a, b)is not the same thing as
a*b. Whenndarray.__mul__callsnp.multiplyit's too late to decide that in fact it should havereturned
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.multiplyreturning 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 inndarray.__mul__and not in the ufuncnp.multiplywhich 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 commentedon 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 theufunccouldn't just pass onNotImplementedif 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:
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 commentedon 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 commentedon 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 commentedon Jun 19, 2015
I'm closing this one, since it is, effectively, a duplicate of #5844.