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

convert PyUntypedArray to Bound API #411

Merged
merged 1 commit into from
Mar 16, 2024
Merged

Conversation

Icxolu
Copy link
Contributor

@Icxolu Icxolu commented Mar 11, 2024

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.

src/array.rs Outdated Show resolved Hide resolved
src/untyped_array.rs Outdated Show resolved Hide resolved
src/untyped_array.rs Outdated Show resolved Hide resolved
src/untyped_array.rs Outdated Show resolved Hide resolved
@davidhewitt
Copy link
Member

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
Copy link
Member

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

Icxolu commented Mar 12, 2024

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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.

src/untyped_array.rs Outdated Show resolved Hide resolved
src/array.rs Outdated Show resolved Hide resolved
@adamreichold adamreichold merged commit 9517ed9 into PyO3:main Mar 16, 2024
33 of 35 checks passed
@Icxolu Icxolu deleted the pyuntypedarray branch March 16, 2024 17:23
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