Skip to content

MAINT: Update NEP-30#14257

Merged
rgommers merged 3 commits intonumpy:masterfrom
pentschev:update-nep30
Sep 7, 2019
Merged

MAINT: Update NEP-30#14257
rgommers merged 3 commits intonumpy:masterfrom
pentschev:update-nep30

Conversation

@pentschev
Copy link
Copy Markdown
Contributor

Update NEP-30, as per discussion in mailing list.

@charris charris changed the title Update NEP-30 MAINT: Update NEP-30 Aug 12, 2019
@rgommers
Copy link
Copy Markdown
Member

Expanding on my comment on the mailing list that @shoyer agreed with, how about adding something like:

Code that uses np.duckarray is meant for supporting other ndarray-like objects that "follow the NumPy API". That is an ill-defined concept at the moment - every known library implements the NumPy API only partly, and many deviate intentionally in at least some minor ways. This cannot be easily remedied, so for users of duckarray we recommend the following strategy: check if the NumPy functionality used by the code that follows your use of duckarray is present in Dask, CuPy and Sparse. If so, it's reasonable to expect any duck array to work here. If not, we suggest you indicate in your docstring what kinds of duck arrays are accepted, or what properties they need to have.

@pentschev
Copy link
Copy Markdown
Contributor Author

@rgommers thanks for taking the time to write down your suggestion, I agree with it and my apologies, it looks like I misunderstood your argument at the mailing list. The new commit adds that, let me know what you think.

@pentschev
Copy link
Copy Markdown
Contributor Author

Thanks @shoyer for the great review, trying to simplify the example I ended up simplifying it so much that it was just concatenating. All should be fixed now in the latest commit. :)

@rgommers rgommers merged commit c280ab6 into numpy:master Sep 7, 2019
@rgommers
Copy link
Copy Markdown
Member

rgommers commented Sep 7, 2019

This all LGTM, should have been merged a while ago. Thanks @pentschev and @shoyer

@pentschev
Copy link
Copy Markdown
Contributor Author

Thanks @rgommers for merging, I didn't follow-up on this before because we still had the naming discussion open, but this can always be addressed on a new PR.

@rgommers
Copy link
Copy Markdown
Member

Indeed. This is still a significant improvement as is and status is still Draft, so good to have the rendered version in the docs up-to-date

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants