Skip to content

use datetime.fold to distinguish ambiguous datetimes when converting#4791

Merged
Icxolu merged 2 commits intoPyO3:mainfrom
bschoenmaeckers:chrono-ambiguous
Dec 16, 2024
Merged

use datetime.fold to distinguish ambiguous datetimes when converting#4791
Icxolu merged 2 commits intoPyO3:mainfrom
bschoenmaeckers:chrono-ambiguous

Conversation

@bschoenmaeckers
Copy link
Copy Markdown
Member

fixes #4722

@bschoenmaeckers
Copy link
Copy Markdown
Member Author

bschoenmaeckers commented Dec 12, 2024

We should probably do this for the IntoPyObject case as well. Current implementation will always set fold to 0.

@bschoenmaeckers bschoenmaeckers force-pushed the chrono-ambiguous branch 6 times, most recently from 0c0c588 to 36be8a8 Compare December 13, 2024 10:25
Copy link
Copy Markdown
Member

@Icxolu Icxolu left a comment

Choose a reason for hiding this comment

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

We should probably do this for the IntoPyObject case as well. Current implementation will always set fold to 0.

Thanks! Agreed. Implementation looks good to me. Perhaps we can add a round-trip test converting (non)ambiguous DateTime<Tz> into Python, then extracting them back and make sure they are the same? What do you think?

@bschoenmaeckers
Copy link
Copy Markdown
Member Author

We should probably do this for the IntoPyObject case as well. Current implementation will always set fold to 0.

Thanks! Agreed. Implementation looks good to me. Perhaps we can add a round-trip test converting (non)ambiguous DateTime<Tz> into Python, then extracting them back and make sure they are the same? What do you think?

Current test verifies this behaviour as well

Copy link
Copy Markdown
Member

@Icxolu Icxolu left a comment

Choose a reason for hiding this comment

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

Current test verifies this behaviour as well

Oops, I must have overlooked that part.

@Icxolu Icxolu added this pull request to the merge queue Dec 16, 2024
Merged via the queue into PyO3:main with commit b17de4f Dec 16, 2024
@bschoenmaeckers
Copy link
Copy Markdown
Member Author

Great! Thanks for looking at my changes!

davidhewitt pushed a commit that referenced this pull request Jan 3, 2025
#4791)

* use `datetime.fold` to distinguish ambiguous datetimes when converting

* Set correct fold when converting to ambiguous `chrono::DateTime<Tz>`
davidhewitt pushed a commit that referenced this pull request Jan 11, 2025
#4791)

* use `datetime.fold` to distinguish ambiguous datetimes when converting

* Set correct fold when converting to ambiguous `chrono::DateTime<Tz>`
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.

DateTime<Tz>'s implementation for FromPyObject does not consider Pythons's datetime.fold attribute

2 participants