search

PyO3 / rust-numpy

PyO3-based Rust bindings of the NumPy C-API
BSD 2-Clause "Simplified" License
1.11k stars 106 forks source link

convert `PyUntypedArray` to `Bound` API #411

Closed Icxolu closed 6 months ago

Icxolu commented 7 months ago

Following #410

This migrates PyUntypedArray to the Bound API. This introduces PyUntypedArrayMethods to implement the methods on Bound<PyUntypedArray>. Additionally this also implements the trait on Bound<PyArray<T, D>>, via as_untyped, because we will not be able to provide a Deref implementation anymore, due to orphan rules and the deref to Bound<PyAny>. This seemed like the most reasonably option to keep the same ergonomics.

davidhewitt commented 6 months ago

we will not be able to provide a Deref implementation anymore, due to orphan rules and the deref to Bound. This seemed like the most reasonably option to keep the same ergonomics.

Should we be changing the PyO3 deref implementation to make it possible for downstream to decide the target type? This might be relevant in class hierarchies, for example.

Alternatively we could experiment with removing the deref-to-pyany blanket and instead implement PyAnyMethods for all types Bound<T>.

davidhewitt commented 6 months ago

Alternatively we could experiment with removing the deref-to-pyany blanket and instead implement PyAnyMethods for all types Bound.

I guess the problem with doing that is that things like list.iter() become ambiguous between the two traits, so the answer is that that approach doesn't work.

Icxolu commented 6 months ago

Should we be changing the PyO3 deref implementation to make it possible for downstream to decide the target type? This might be relevant in class hierarchies, for example.

It might be a good idea experimenting with that. I don't think it would be a huge deal here, nice to have, but not a deal breaker either. Maybe we can add an associated type to DerefToPyAny, which defaults to PyAny? For class hierarchies this would probably involve pyo3s macro code.

davidhewitt commented 6 months ago

So, given that the deref idea doesn't work out, I guess that the design as proposed here is the right way to proceed?

adamreichold commented 6 months ago

So, given that the deref idea doesn't work out, I guess that the design as proposed here is the right way to proceed?

Yes, I just did not find time to complete the review here yet.

davidhewitt commented 6 months ago

No problem, I have also had a disrupted week. I'm hoping to go through the list of actions for 0.21 and see what remains before the final release later.