Skip to content
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

MAINT, DOC: make np._from_dlpack public #21145

Merged
merged 10 commits into from
Mar 10, 2022
Merged

Conversation

tirthasheshpatel
Copy link
Contributor

@tirthasheshpatel tirthasheshpatel commented Mar 3, 2022

fixes gh-20743

This PR:

  • Makes np._from_dlpack public.
  • Adds docs for the DLPack protocol in the user guide section "Interpolability with NumPy"

cc @seberg @mattip @leofang

@melissawm this might interest you :)

@charris
Copy link
Member

charris commented Mar 8, 2022

Won't this break backwards compatibility? Perhaps we should keep both names for a bit while deprecating the underscored version.

@tirthasheshpatel
Copy link
Contributor Author

Won't this break backwards compatibility?

I didn't expect backward compatibility would be a concern for private methods (_from_dlpack wasn't even documented before). The reason we kept _from_dlpack private was that the protocol wasn't formally documented anywhere and there were a few bugs. Do you think it's OK not to worry about backward compatibility given the above reasons? If not, I can keep both the versions :)

doc/source/reference/arrays.interface.rst Show resolved Hide resolved
doc/source/user/basics.interoperability.rst Outdated Show resolved Hide resolved
doc/source/user/basics.interoperability.rst Outdated Show resolved Hide resolved
doc/source/user/basics.interoperability.rst Show resolved Hide resolved
doc/source/user/basics.interoperability.rst Show resolved Hide resolved
doc/source/user/basics.interoperability.rst Outdated Show resolved Hide resolved
doc/source/user/basics.interoperability.rst Outdated Show resolved Hide resolved
doc/source/user/basics.interoperability.rst Outdated Show resolved Hide resolved
numpy/core/_add_newdocs.py Show resolved Hide resolved
@mattip mattip added the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label Mar 9, 2022
Co-authored-by: Matti Picus <matti.picus@gmail.com>
@mattip
Copy link
Member

mattip commented Mar 9, 2022

Could you add a note in doc/release/upcoming_changes? There is a README there which should explain how to add a new_feature.rst

@tirthasheshpatel
Copy link
Contributor Author

Added the function to upcoming changes @mattip!

@mattip
Copy link
Member

mattip commented Mar 9, 2022

LGTM. Let's wait a day or so to see if the mailing list discussion leads to changes.

The rendered documentation can be seen here

@tirthasheshpatel
Copy link
Contributor Author

The one failure seems unrelated.

@mattip mattip merged commit 3135e1f into numpy:main Mar 10, 2022
@mattip
Copy link
Member

mattip commented Mar 10, 2022

Thanks @tirthasheshpatel

@tirthasheshpatel tirthasheshpatel deleted the fix-gh20743 branch March 10, 2022 16:15
@tirthasheshpatel
Copy link
Contributor Author

Thanks @mattip @charris @leofang for the reviews!

@rgommers rgommers added this to the 1.23.0 release milestone Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
03 - Maintenance 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: Make from_dlpack() a public API under the main module
5 participants