-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
NEP: Update NEP-18 to include the __skip_array_function__
attribute.
#13305
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
Conversation
Fix GH12974 This includes two minor changes to NEP-18 that will need to be implemented in NumPy: - The ``__numpy_implementation__`` attribute is documented and officially supported as a way to defer to NumPy's own implementation. - I have noted that dispatcher functions should be given the same name as implementations, which should result in more interprettable error messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good. But there is the bike shed: I propose to just call it __implementation__
- partially because our other methods do not refer to numpy
either, and partially because I think we would like to encourage eventually, e.g., scipy to wrap its functions similarly (already works for all its special functions with __array_ufunc__
).
What I wanted to invoke with the name (Either way, this is probably worth clarifying in the text.) |
I'm not 100% sure that is right: suppose DASK wants to make its functions more general, is its default implementation a On the other hand, I guess that is what you are getting add: with I do wonder if for this case we should recommend to cast oneself to |
Dask defines it's own data structures, so it's generic implementations would be something like
Maybe Let me try adding some clarifying language about the intent of
We've discussed this before, but this doesn't work as a fall-back in in |
OK, yes, should have remembered the problem with If we're abbreviating, probably |
I prefer |
This should probably be accompanied by consensus on the mailing list, as per governance rules. |
I'm not sure this comparison is entirely apt. So far (until this PR) the I guess the critical question is whether it makes sense to distinguish between array function implementations intended for different types. For example, consider a scipy function that is implemented by coercing its arguments to sparse arrays, e.g.,
(1) would preserve existing behavior for (2) feels a little strange -- this implementation is not NumPy array specific. (Though it is what should be used on NumPy arrays.) (3) could make sense, but there's no way NumPy is going to know to call a The advantage of class SparseArray:
def __array_function__(self, func, types, args, kwargs):
if not all(issubclass(t, (SparseArray, np.ndarray) for t in types):
return NotImplemented
sparse_func = getattr(func, '__sparse_implementation__', None)
if sparse_func is not None:
return sparse_func(*args, **kwargs)
# look elsewhere for implementations, or return NotImplemented In contrast, |
Thinking more about this, I'm pretty firmly of the opinion that we should stick with This attribute is current used for three things:
For all these use cases, the name "numpy" is more descriptive than "array." The situation reminds me of the mistake that is the name of the Yes, assuming the existence of a The generic alternative would be to guard access to class ndarray:
def __array_function__(self, func, types, args, kwargs):
if not all(issubclass(t, ndarray) for t in types):
# Defer to any non-subclasses that implement __array_function__
return NotImplemented
implementation = getattr(func, '__numpy_implementation__', None)
if implementation is None:
return NotImplemented
return implementation(*args, **kwargs) But I think this would be a little much. It is not too onerous to expect that libraries that reuse NumPy's dispatching protocol could also at least implement a dummy version of |
Thinking a bit more, I agree with @shoyer. It does make sense to "separate the interface from implementation" in a sense. |
My original suggestion was plain |
I emailed the mailing list: https://mail.python.org/pipermail/numpy-discussion/2019-April/079317.html |
return NotImplemented | ||
if not all(issubclass(t, ndarray) for t in types): | ||
# Defer to any non-subclasses that implement __array_function__ | ||
return NotImplemented |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if all the other types
are python builtins?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They don't implement __array_function__
so the old behavior still applies... But I believe that's lacking in this code example, but is implemented in the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shoyer Does the example need updating?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's all okay... It'll fall through, so the example is fine.
__skip_array_function__
attribute.
I think it was agreed that this is ready after the name change. Comments? |
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Are NEPs still being automatically updated on numpy.org? The old version of NEP-18 does not seem to have been updated yet: http://www.numpy.org/neps/nep-0018-array-function-protocol.html |
Very strange. The built documents are pushed to a repo, and the nep there is the updated version. |
This reverts most of the changes from numpyGH-13305, and adds a brief discussion of ``__skip_array_function__`` into the "Alternatives" section. We still use NumPy's implementation of the function internally inside ``ndarray.__array_function__``, but I've given it a new name in the NEP (``_implementation``) to indicate that it's a private API.
Fixes #12974
This includes two minor changes to NEP-18 that will need to be implemented in NumPy:
__numpy_implementation__
attribute is documented and officiallysupported as a way to defer to NumPy's own implementation.
implementations, which should result in more interprettable error messages.