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

Handling of leap seconds in chrono feature is probably wrong #3424

Closed
davidhewitt opened this issue Aug 31, 2023 · 0 comments · Fixed by #3458
Closed

Handling of leap seconds in chrono feature is probably wrong #3424

davidhewitt opened this issue Aug 31, 2023 · 0 comments · Fixed by #3458
Labels

Comments

@davidhewitt
Copy link
Member

While investigating our current CI break and #3266...

At the moment there is some code in our chrono into-python conversion which checks for leap-seconds, and in the case of a leap-second, sets fold=1 for the resultant Python datetime object.

Similarly on the FromPyObject leg, if fold=1 we convert the result back into a leap-second. Although it makes our conversion above round-trip, this is almost certainly wrong in the general case of #3266, because fold is meant to be used for DST, not leap seconds.

NB that datetime does not even support leap seconds! python/cpython#67762

I think we should remove this attempt to round-trip leap-seconds. Given that the only option to fail on the to-python leg is to panic (pending #1813), I'm slightly leaning towards clamping the leap-second and raising a warning. Even with fallible IntoPy I wonder if it might be better to do that than fail for a very select edge-case which (if we ever get another leap-second) will probably be at midnight UTC.

Opinions welcome, I will probably try to push an implementation / fix tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant