Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1194 +/- ##
==========================================
+ Coverage 91.68% 91.71% +0.02%
==========================================
Files 27 27
Lines 4691 4693 +2
==========================================
+ Hits 4301 4304 +3
+ Misses 390 389 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixes #1192 (but uncovers another bug)
|
If you aren't in a rush to merge this, I will take a look later this week :) |
tomasr8
left a comment
tomasr8
left a comment
There was a problem hiding this comment.
Not much of a c++ person, but the tz changes look reasonable to me 👍
| tz = timezone_getter("Pacific/Honolulu") | ||
| dt = _localize(tz, datetime(2025, 3, 4, 13, 53, tzinfo=UTC)) | ||
| assert dates.format_datetime(dt, "YYYY-MM-dd H:mm z", locale="en_US") == "2025-03-04 3:53 HST" | ||
| assert dates.format_datetime(dt, "YYYY-MM-dd H:mm z", locale="en_GB") == "2025-03-04 3:53 GMT-10" |
There was a problem hiding this comment.
I did some digging and we get Hawaii-Aleutian Standard Time instead of GMT-10 here because of this fallback to the long tz name:
Lines 659 to 662 in 2b93a4a
When I get rid of it, get_timezone_gmt is used instead.
There was a problem hiding this comment.
Ah yeah, I should've mentioned I also found the culprit, but it's out of scope for this PR 😅
2 participants
Fixes #1192 – in a way:
As far as I can tell, there's no well-specified operation called "get timezone name" in the TR35 spec, so doing what we do here (ending up using the location) is... okay for now, at least better than returning ∅∅∅.
However, this uncovers a bug in how we fallback timezone formatting in this specific case, for which I added an xfailing test.
There's also a new janky C++ tool that lets one format times in various TZes and locales and formats using ICU4C.