Skip to content

Ufunc overrides should always raise TypeError if any arguments set __array_ufunc__ = None #9011

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
shoyer opened this issue Apr 27, 2017 · 6 comments

Comments

@shoyer
Copy link
Member

shoyer commented Apr 27, 2017

Following up on our discussion in #8247 (comment), we need to switch ufunc overrides to always raise TypeError if any arguments set __array_ufunc__ = None. The avoids potential for confusion with x + y and np.add(x, y) yielding different results.

This is a blocker for the 1.13.0 release.

@mhvk
Copy link
Contributor

mhvk commented Apr 27, 2017

I'm still not sure I entirely agree with this: at least my original intention was for __array_ufunc__ = None to mean that ndarray could not deal with the class, i.e., it was meant as a replacement for setting __array_priority__ to a very high value. I'm not sure we really need to protect implementers of their own __array_ufunc__ from making mistakes, and thus also preventing them from possibly handling such objects. (Though in practice it may just mean defining a few binary methods.)

@charris
Copy link
Member

charris commented Apr 27, 2017

@mhvk There wasn't a formal vote, but my impression was that the majority thought is should simply raise a TypeError in this situation. In practice, I doubt it will make much difference, so the simplest behavior has much to recommend it. The use of big __array_priority__ values was a workaround that abused the functionality as there was no other way to achieve the desired behaviour.

@njsmith
Copy link
Member

njsmith commented Apr 29, 2017

@mhvk: Consider np.add(a, b), where a sets __array_ufunc__ = None. In current master, this will always raise an error, regardless of how b implements __array_ufunc__. So I don't think there's much value in making it possible for np.add(b, a) to return something different?

(Well, I guess if a.__array_ufunc__ is None and b is a proper subclass of a that defines a real __array_ufunc__ method then np.add(a, b) could work but ughghgg my head hurts. It's just easier to think about everything if __array_ufunc__ = None means it doesn't participate in ufuncs, with no complicated exceptions that need documenting.)

@mhvk
Copy link
Contributor

mhvk commented Apr 29, 2017

In current master, if b also implements __array_ufunc__ and can deal with a, it will in fact work (if I'm wrong in this, then I implemented it wrong...). I have to admit I cannot think of a good example of where this will be useful; it is more a feeling that a class should not be allowed to determine that something doesn't work if it also involves another class; all it can say is that it itself cannot handle ufuncs -- i.e., __array_ufunc__ = None is just a short-cut for return NotImplemented which allows one to avoid calling it -- and for ndarray we draw the logical conclusion that we get a TypeError for the ufunc and NotImplemented for the operators.

Here, I think part of the differences arises from my feeling that the ufunc module should effectively be more like operator, with the actual implementation in C considered part of ndarray, not of the ufunc itself (I really hope to do this in follow-up...). This means that the ufunc module always tries all operands, even if one of them already signals ahead it cannot do it.

@mhvk
Copy link
Contributor

mhvk commented Apr 29, 2017

Continuing on the above: there is no direct precedent from python, but I think one should treat __array_ufunc__ = None the same as the absence of an operator special method, which in python is treated as equivalent to returning NotImplemented, i.e., it does not preclude another class from having the reverse method and doing something. (For ufuncs we cannot do the same thing since obviously the absence of __array_ufunc__ simply invokes the legacy mechanism.)

In the end, I do think it boils down to the question of whether one treats the ufunc module as a standard interface that numpy provides to evaluate mathematical functions, i.e., whether as I suggested above it is similar to operator (arguably, math should provide overrides, but that would seem a bit of a pipe dream...).

@mhvk
Copy link
Contributor

mhvk commented Apr 30, 2017

As noted in #9014, I'd still prefer sticking with what we have, but definitely do not feel very strongly about it. So, fine to go with the stricter rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants