Skip to content

MAINT, DOC: make np._from_dlpack public#21145

Merged
mattip merged 10 commits intonumpy:mainfrom
tirthasheshpatel:fix-gh20743
Mar 10, 2022
Merged

MAINT, DOC: make np._from_dlpack public#21145
mattip merged 10 commits intonumpy:mainfrom
tirthasheshpatel:fix-gh20743

Conversation

@tirthasheshpatel
Copy link
Copy Markdown
Contributor

@tirthasheshpatel tirthasheshpatel commented Mar 3, 2022 •
edited
Loading

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 :)

tirthasheshpatel
tirthasheshpatel commented Mar 8, 2022
View reviewed changes
@charris
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

tirthasheshpatel commented Mar 8, 2022

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 :)

mattip
mattip reviewed Mar 8, 2022
View reviewed changes
mattip
mattip reviewed Mar 9, 2022
View reviewed changes
@mattip mattip added the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label Mar 9, 2022
@tirthasheshpatel @mattip
DOC: from_dlpack(x) to numpy.from_dlpack
464a124 
Co-authored-by: Matti Picus <matti.picus@gmail.com>
@mattip
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

tirthasheshpatel commented Mar 9, 2022

Added the function to upcoming changes @mattip!

@mattip
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

tirthasheshpatel commented Mar 9, 2022

The one failure seems unrelated.

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

mattip commented Mar 10, 2022

Thanks @tirthasheshpatel

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

tirthasheshpatel commented Mar 10, 2022

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

Reviewers

@mattip mattip mattip approved these changes

+1 more reviewer

@leofang leofang leofang left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

03 - Maintenance 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes

Projects

None yet

Milestone

1.23.0 release

Development

Successfully merging this pull request may close these issues.

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

5 participants

@tirthasheshpatel @charris @mattip @leofang @rgommers