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

Convert DateTime::with_nanosecond to return Result #1520

Merged
merged 2 commits into from
Mar 18, 2024

Conversation

pitdicker
Copy link
Collaborator

As I see it DateTime::with_nanosecond is one of the few methods on DateTime that does not need to return an error from the TimeZone trait and does not need to return a MappedLocalTime.

We assume in some places, like the platform code of Local, that time zone transitions happen on second boundaries. That seems entirely reasonable to me and fits with reality. There is also some similarity to how we assume offsets to be a round number of seconds from UTC.

In the IANA time zone database there are some time zone offsets from the 19th century that have an offset with fractional seconds. Those are based on the longitude of a city or mean of the country. The fractional part is included with a comment, and the shipped data uses round seconds. Necessarily if anyone takes those fractional offsets serious, they would need a transitions at a fractional second at some point when the country aligns from local mean time to round offsets from UTC.

If we assume there are no subsecond transitions DateTime::with_nanosecond can just assume the entire second has the same offset from UTC.

Copy link

codecov bot commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.01%. Comparing base (d286f93) to head (7c53bd5).

Additional details and impacted files
@@           Coverage Diff           @@
##            0.5.x    #1520   +/-   ##
=======================================
  Coverage   94.01%   94.01%           
=======================================
  Files          37       37           
  Lines       16830    16834    +4     
=======================================
+ Hits        15823    15827    +4     
  Misses       1007     1007           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pitdicker pitdicker merged commit 7979e88 into chronotope:0.5.x Mar 18, 2024
35 checks passed
@pitdicker pitdicker deleted the datetime_with_nanos branch March 18, 2024 10:50
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.

2 participants