MAINT: cleanup and better document core/overrides.py #12042
Conversation
I think this organization makes a little more sense, especially when thinking about what the functions we want to write in C should look like.
numpy/core/overrides.py
Outdated
| def array_function_override(overloaded_args, func, types, args, kwargs): | ||
| """Call __array_function__ implementations.""" | ||
| def array_function_implementation_or_override( | ||
| implementation_func, api_func, dispatcher, args, kwargs): |
There was a problem hiding this comment.
Might it make more sense to call the first one ndarray_implementation? That is what it would be if we actually worked via ndarray.__array_ufunc__. A nice aspect of that is that one could rename api_func to function.
Or perhaps just implementation and function?
There was a problem hiding this comment.
How about implementation and public_api?
numpy/core/overrides.py
Outdated
|
|
||
| if result is not NotImplemented: | ||
| return result | ||
|
|
||
| raise TypeError('no implementation found for {} on types that implement ' | ||
| '__array_function__: {}' | ||
| .format(func, list(map(type, overloaded_args)))) | ||
| .format(api_func, list(map(type, overloaded_args)))) |
There was a problem hiding this comment.
Only noticing this now, but wouldn't it make sense to just print types in the error message? This will include ndarray if an array was present, but I think that is logically correct: in a way, its __array_function__ has been (implicitly) tried, and ti cannot handle it.
There was a problem hiding this comment.
I'm would lean against this since we don't actually call ndarray.__array_function__.
There was a problem hiding this comment.
We only skip it because we know its answer! But not a big deal either way.
There was a problem hiding this comment.
We also skip it because ndarray.__array_function__ calls the public API function rather than the implementation function, so we need to avoid the infinite loop :)
| def decorator(implementation): | ||
| @functools.wraps(implementation) | ||
| def public_api(*args, **kwargs): | ||
| relevant_args = dispatcher(*args, **kwargs) |
There was a problem hiding this comment.
I moved the call to dispatcher(*args, **kwargs) into array_function_dispatcher() to leave things slightly more flexible for array_function_implementation_or_override(), e.g., for functions with complex argument parsing such as np.einsum().
|
Thanks Stephan. |
I think this organization makes a little more sense, especially when thinking
about what the functions we want to write in C should look like.
CC @mhvk
xref #12028