Skip to content

Conversation

davidhewitt
Copy link
Member

This is a rebuild of #5178 which uses a private helper in FromPyObject to perform the specialized conversion.

@bschoenmaeckers
Copy link
Member

Tnx for continuing the work!

@davidhewitt
Copy link
Member Author

@Icxolu are you happy with this given your alternative suggestion in #5507 (comment) ?

@Icxolu
Copy link
Contributor

Icxolu commented Oct 13, 2025

I'm not at my keyboard this week, so i just skimmed through it. I'm fine with the change and I agree that my suggestion is probably a much bigger performance hit than this. I just think than in general we should try to minimize such specializations because it makes it harder to follow how the extraction works and harder to maintain if too many special methods crop up.

@davidhewitt
Copy link
Member Author

I just think than in general we should try to minimize such specializations because it makes it harder to follow how the extraction works and harder to maintain if too many special methods crop up.

Completely agree with this, I was sad to add a second special case to FromPyObject. My hope is that we won't add another.

@davidhewitt davidhewitt added this pull request to the merge queue Oct 15, 2025
@davidhewitt davidhewitt removed this pull request from the merge queue due to a manual request Oct 15, 2025
@davidhewitt davidhewitt enabled auto-merge October 15, 2025 19:03
@davidhewitt davidhewitt added this pull request to the merge queue Oct 15, 2025
Merged via the queue into PyO3:main with commit 9a213ae Oct 15, 2025
41 of 42 checks passed
@davidhewitt davidhewitt deleted the chrono-local-2 branch October 15, 2025 20:33
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