Skip to content

BUG __numpy_ufunc__ does not properly deal with NotImplemented #5074

Closed

Description

@mhvk

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

Activity

  1. pv commented on Sep 15, 2014

    @pv
    Member

    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
    
  2. pv commented on Sep 15, 2014

    @pv
    Member

    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.

  3. mhvk commented on Sep 15, 2014

    @mhvk
    ContributorAuthor

    @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)?

  4. pv commented on Sep 15, 2014

    @pv
    Member

    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).

  5. mhvk commented on Sep 15, 2014

    @mhvk
    ContributorAuthor

    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?

  6. mhvk commented on Sep 15, 2014

    @mhvk
    ContributorAuthor

    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.

  7. pv commented on Sep 15, 2014

    @pv
    Member

    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...

  8. mhvk commented on Jun 19, 2015

    @mhvk
    ContributorAuthor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions