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

Add AsRefSource to PyNativeType. #3653

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

davidhewitt
Copy link
Member

This adds an associated type AsRefSource to PyNativeType.

This allows type inference in the reverse direction &T -> Py<T>, which is particularly useful for &PyCell<T> -> Py2<T> . It also helps for forwarding our gil-ref methods to the new traits, e.g. by solving the inference problem in #3652

Really this is most helpful when we're in a transition phase where users might be using Py2::borrowed_from_gil_ref themselves. So I'm also happy to decide that we don't merge this until we're ready to release Py2 as a whole, and for now we just continue to add type annotations while we're landing the groundwork internally.

Copy link
Member

@adamreichold adamreichold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should hold off on the quality-of-life improvements for internal work as it makes this faster.

I am currently not seeing a downside besides a general unease with more coupling within our already complex trait landscape. Is there anything in particular that one should be aware of?

@davidhewitt
Copy link
Member Author

Thanks; yes this makes migration work nicer for sure. I think the only downside is that manual implementors of PyNativeType will need to add this associated type for very little immediate benefit, depending on whether the Py2 stuff makes it into 0.21 (still unsure how fast we're going to manage to go).

In the longer term, I think we can throw away the whole PyNativeType trait when we get rid of the pool and gil refs. So we have to ride over a peak of complexity in the short term and can then simplify again.

@davidhewitt davidhewitt added this pull request to the merge queue Dec 14, 2023
@davidhewitt
Copy link
Member Author

(it should be noted that I don't expect there to be downstream manual implementors of PyNativeType, but you never know...)

Merged via the queue into PyO3:main with commit 97cf9b8 Dec 14, 2023
37 checks passed
@davidhewitt davidhewitt deleted the native-type-source branch December 14, 2023 23:10
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.

2 participants