Skip to content

Conversation

@Icxolu
Copy link
Contributor

@Icxolu Icxolu commented Mar 13, 2024

This changes the bound of PyAny::extract to use FromPyObjectBound instead of FromPyObject. This allows extracting borrowed types like &str or &[u8] using the gil-ref API, even if the gil-refs feature is disabled.

To allow this, FromPyObjectBound::from_py_object_bound is switched to take a Borrowed instead of &Bound. This also means that the FromPyObjectBound implemetations can now be unconditionally. To not break fully qualified syntax (FromPyObject::extract) these are kept with gil-refs enabled.

Fixes #3958

@Icxolu Icxolu added the CI-skip-changelog Skip checking changelog entry label Mar 13, 2024
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 13, 2024

CodSpeed Performance Report

Merging #3959 will not alter performance

Comparing Icxolu:extract (07386db) with main (989d2c5)

Summary

✅ 67 untouched benchmarks

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.

Making PyAny::extract work the same way as the other .extract() methods seems like a good consistency change. Because FromPyObjectBound has a blanket from FromPyObject, I would hope that it means that users depending on FromPyObject as a trait bound in generic code wouldn't experience breaking changes.

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.

This looks good to me, thanks!

@davidhewitt davidhewitt added this pull request to the merge queue Mar 15, 2024
Merged via the queue into PyO3:main with commit 5c86dc3 Mar 15, 2024
@Icxolu Icxolu deleted the extract branch March 15, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI-skip-changelog Skip checking changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better document how to migrate borrowing extract() calls

2 participants