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

Remove usage of AsPyPointer in IntoPy<PyObject> trait implementation #3393

Merged
merged 1 commit into from
Aug 18, 2023

Conversation

alex
Copy link
Contributor

@alex alex commented Aug 16, 2023

No description provided.

@alex
Copy link
Contributor Author

alex commented Aug 16, 2023

FWIW I believe the set of types here is comprehensive, looking at https://docs.rs/pyo3/latest/pyo3/conversion/trait.AsPyPointer.html#implementors:

  • The first gazillion all implement AsRef<PyAny>
  • PyRef and PyMutRef implement IntoPy<Py<PyAny>> themselves. Do we need an impl for &PyRef and &PyMutRef?
  • Py is handled here
  • PyCell implements AsRef<PyAny>

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

While I think &PyRef and &PyRefMut are extremely rare cases, given the amount of churn we might create for users with all this change to AsPyPointer and IntoPyPointer, it seems fairer to users if we add those impls too. They did exist under the old blanket impl, after all.

@alex
Copy link
Contributor Author

alex commented Aug 17, 2023

Easy enough.

@alex
Copy link
Contributor Author

alex commented Aug 17, 2023

Done

src/instance.rs Outdated Show resolved Hide resolved
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks! One more log on the AsPyPointer bonfire. 😂

@davidhewitt davidhewitt added this pull request to the merge queue Aug 18, 2023
Merged via the queue into PyO3:main with commit c03c787 Aug 18, 2023
31 of 32 checks passed
@alex alex deleted the drop-other-trait branch August 18, 2023 14:12
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.

None yet

3 participants