NEP: Add NEP-35 instructions on reading like= downstream #17725
Conversation
|
cc @hameerabbasi @seberg @eric-wieser @shoyer @rgommers @quasiben @jakirkham @mrocklin @leofang Also @jni @ilayn @mattip @Qiyu8 as you have provided feedback on the previous update to this NEP in #17093 , but don't feel obligated to do so again. :) |
| # If ``like`` is contained in ``da_func``'s signature, add ``like=self`` | ||
| # to the kwargs dictionary. | ||
| if 'like' in kwonlyargs: | ||
| kwargs['like'] = self |
There was a problem hiding this comment.
Seeing the dance with getfullargspec (which obviously could be replaced with something else though), I actually wonder if the always pass solution (forcing downstream to drop it if they do not want to add like= to their own wrapper) isn't the better solution after all? Maybe we really need some opinions how downstream authors feel about being forced to drop the kwarg, @hameerabbasi is one of them, @shoyer, or @mhvk may also have opinions?
The issue is whether like=arrlike is passed as a kwarg to __array_function__ or (as currently the case) dropped and access to arrlike would have to be achieved by falling back to self.
There was a problem hiding this comment.
Seeing the dance with
getfullargspec(which obviously could be replaced with something else though), I actually wonder if the always pass solution (forcing downstream to drop it if they do not want to addlike=to their own wrapper) isn't the better solution after all? Maybe we really need some opinions how downstream authors feel about being forced to drop thekwarg, @hameerabbasi is one of them, @shoyer, or @mhvk may also have opinions?
FWIW, I would rather not force downstream libraries to drop like manually. The solution here is, IMO, much more convenient for the majority of libraries (e.g., CuPy, that won't need like= explicit nor have to do any changes on their side), while still providing at least a couple of different alternatives to libraries that may have a use for like= (e.g., Dask). Take my opinion with a grain of salt, but I'm here trying to chime in as a maintainer for Dask's __array_function__, and potentially to CuPy as well.
There was a problem hiding this comment.
@seberg - for astropy's Quantity, I think we could fairly easily deal with it either way, though having the like argument might be slightly more convenient with the current setup. But being an ndarray subclass, we're not really the target audience for the like argument anyway: in the end, we generally just wrap the numpy function call instead of calling something else entirely. I can definitely see that for dask/cupy it would be easier to just call their own routines, passing all the arguments (and since it is guaranteed that the argument of like is self, that should be OK; while arguably __array_function__ could have been a class method, I think we also use that it is not). That said, code simplicity does count!
There was a problem hiding this comment.
Thanks, I was just curious what you think. I don't mind this, and we could make it more convenient in the future by providing the list of these function (e.g. as a special attribute on the public symbol).
There was a problem hiding this comment.
I don't mind having this list either, on the contrary, I think it would be useful. Would you prefer to have that still in 1.20 or should we leave it as is while this is experimental and, assuming we'll transition this feature to stable, try to address it in 1.21?
There was a problem hiding this comment.
I think we consider adding it even now if it is useful to you. My gutfeeling would be making the example effectively: if getattr(public_api, "__array_function_like_is_self__", False): ... with some bike shedding on the name?
It doesn't really strike me as a difficult decision to be made. Although we could add a test in NumPy that such an attribute is set if inspect detects a like argument.
There was a problem hiding this comment.
Ah, I just realized that tagging something on to a C-defined function may be a bit too involved...
There was a problem hiding this comment.
Yeah, exactly. I think we can't inspect functions like np.array, but maybe there's some way I don't know of.
There was a problem hiding this comment.
Thanks Peter. The changes look good to me, and are mostly in the technical "appendix". With or without changes, I will plan to merge it in the next days.
I currently assume we will change the like= argument to be strict. I am not sure if that requires a real change here, but maybe we could include a snippet explaining that:
def asduckarray(arr):
if hasattr(arr, "__array_function__"):
return arr
return np.asarray(arr)
is a solution for libraries who want to use this, but are currently stuck with the coercing behaviour of np.asarray.
| np.asarray([1, 2, 3], like=da.array(cp.array(()))) | ||
|
|
||
| # Returns a cupy.ndarray | ||
| type(np.asarray([1, 2, 3], like=da.array(cp.array(()))).compute()) |
There was a problem hiding this comment.
Might be nice to paste the output so that the user can read up that the next sentence is true, but just a thought and a tiny one.
There was a problem hiding this comment.
I thought about this, but all the code here is "copy-pasteable", thus I added the output as comments instead. Therefore, I'd rather not add the outputs risking making it more confusing.
doc/neps/nep-0035-array-creation-dispatch-with-array-function.rst
Outdated
Show resolved
Hide resolved
| # If ``like`` is contained in ``da_func``'s signature, add ``like=self`` | ||
| # to the kwargs dictionary. | ||
| if 'like' in kwonlyargs: | ||
| kwargs['like'] = self |
There was a problem hiding this comment.
Thanks, I was just curious what you think. I don't mind this, and we could make it more convenient in the future by providing the list of these function (e.g. as a special attribute on the public symbol).
We could do that, but I'm also wondering if we shouldn't then just introduce NEP-30 and be complete instead of suggesting a downstream implementation, what do you think? |
Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
|
That is true, we could add |
I'm ok with delaying that too, it was just a thought on trying to get everything done in one go. |
|
Well, putting this, we can always fix it up. If |
This PR is a follow-up to a discussion in #17678 where I was attempting to pass
like=downstream when that keyword is available on the implementation. However, thanks to @hameerabbasi pointing this out, we have a much cleaner solution from NumPy's perspective, where no changes are necessary to code. Everything can be done directly within the downstream implementation, and the changes here are only reference instructions on doing that.