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

disable pyo3's gil-refs migration feature #420

Merged
merged 1 commit into from
Mar 30, 2024

Conversation

Icxolu
Copy link
Contributor

@Icxolu Icxolu commented Mar 28, 2024

This disables pyo3s gil-ref migration feature, and fixes all the deprecations that are still left.

One major change to note is in PyArrayDescr::names, which I changed to return a Vec<PyBackedStr> instead of Vec<&str>. I don't think we can support the old signature anymore without gil-refs.

I noticed here that PyBacked* does not implement Debug. Maybe we should add one in pyo3 (which could just delegate to the debug of the Deref::Target). Some PartialEq / Eq impls could also be nice I think. cc @davidhewitt

@davidhewitt
Copy link
Member

I noticed here that PyBacked* does not implement Debug. Maybe we should add one in pyo3 (which could just delegate to the debug of the Deref::Target). Some PartialEq / Eq impls could also be nice I think. cc @davidhewitt

Definitely. I think we could even have PartialEq<&str> or similar. Maybe we should also have implemented Hash, Ord, and PartialOrd?

I also just separately had noticed Send/Sync were missing PyO3/pyo3#4007

These types were definitely a somewhat-last-minute addition that have turned out to be super useful (in particular PyBackedStr) and need some additional love and documentation!

README.md Outdated Show resolved Hide resolved
src/dtype.rs Outdated Show resolved Hide resolved
@Icxolu Icxolu force-pushed the disable-gil-ref-feat branch 2 times, most recently from 1d7fd3d to 23355f8 Compare March 29, 2024 15:02
@adamreichold
Copy link
Member

One more thing I just noticed is that we now have access to Borrowed::to_owned which should allow us to get rid of a FIXME somewhere IIRC.

@Icxolu Icxolu force-pushed the disable-gil-ref-feat branch from 23355f8 to fc43f16 Compare March 30, 2024 09:05
@adamreichold adamreichold merged commit 0b39d09 into PyO3:main Mar 30, 2024
33 of 35 checks passed
@adamreichold
Copy link
Member

@Icxolu So if I am not mistaken this is it to make rust-numpy useful as dependency together with pyo3 0.21? Meaning I will cut a 0.21 release now or is there anything you still want to do before that?

(Regarding my own PR, I don't think the ufunc support is pressing enough to delay the release with the whole PyO3 ecosystem using to the new bound API.)

@Icxolu Icxolu deleted the disable-gil-ref-feat branch March 30, 2024 18:20
@Icxolu
Copy link
Contributor Author

Icxolu commented Mar 30, 2024

I think at this point rust-numpy should be compatible and useful together with pyo3 0.21. The things (I'm aware of) that are left are

  • convert inner, dot and einsum
  • convert pyarray! macro

Both of these should be only additive (introducting _bound variant, and deprecating the old one), so I think it would also be fine to leave them for a point release. I'll leave that up to you.

In any case a big thanks for all the reviewing ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants