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 more IntoPyObject impls #4592

Merged
merged 1 commit into from
Oct 4, 2024
Merged

Conversation

Icxolu
Copy link
Contributor

@Icxolu Icxolu commented Oct 3, 2024

While working through the deprecation of ToPyObject I noticed a few missing IntoPyObject impls.

@Icxolu Icxolu added the CI-skip-changelog Skip checking changelog entry label Oct 3, 2024
@Icxolu Icxolu force-pushed the intopyobject-impls branch from 995bf79 to 3fba5b5 Compare October 3, 2024 08:31
@Icxolu Icxolu force-pushed the intopyobject-impls branch from 3fba5b5 to e01f764 Compare October 3, 2024 08:31
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.

Looks good to me, thanks!

@@ -64,6 +64,17 @@ impl<'py> IntoPyObject<'py> for &OsStr {
}
}

impl<'py> IntoPyObject<'py> for &&OsStr {
Copy link
Member

Choose a reason for hiding this comment

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

All of these && implementations are interesting. I think if we were starting again we wouldn't offer them, but due to the existing ToPyObject implementations they already exist. I wonder if some time in the future we should remove these (presumably would be a micro-optimization to avoid the pointer-to-slice), but doesn't seem worth doing with all the other churn in 0.23

@davidhewitt davidhewitt added this pull request to the merge queue Oct 4, 2024
Merged via the queue into PyO3:main with commit e9a1b9b Oct 4, 2024
42 of 43 checks passed
@Icxolu Icxolu deleted the intopyobject-impls branch October 4, 2024 11:11
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.

2 participants