Repository navigation
Conversation
Both locales list the same day_abbreviation for two different weekdays. Latvian has "pi" for pirmdiena and piektdiena, and Maltese has "Ħ" for Il-Ħamis and Il-Ħadd. The parser resolves ddd with list.index(), which returns the first match, so the second of each pair cannot be parsed: formatting Friday in lv and then parsing it back gives Monday, and the same round trip in mt turns Sunday into Thursday. Latvian already uses the two-letter forms that distinguish the days, so piektdiena takes "pk". Maltese Ħamis and Ħadd only differ from the third letter, so they take "Ħam" and "Ħad"; the locale already mixes lengths for the same reason, with "TL" for Tlieta against "T" for Tnejn.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1380 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 2315 2315
Branches 358 358
=========================================
Hits 2315 2315 ☔ View full report in Codecov by Harness. |
Wkhanex
left a comment
There was a problem hiding this comment.
I checked formatter/parser roundtrips for all seven weekday abbreviations in Latvian and Maltese. The changed Friday/Sunday spellings now select their own weekday rather than a previously duplicated abbreviation, and formatting agrees with parsing.
Checked 3aaaf9ecc969 with standalone Python 3.12.14 checks (28 passing assertions). I haven't run the full upstream suite.
|
A note for whoever picks this up, so the red X doesn't cost anyone time. The On scope, since "why only these two locales" is the obvious question: I checked all 81 locales for weekday or month names and abbreviations that repeat within a single locale. A repeat is unparseable rather than merely untidy, because the parser builds one alternation and the name is looked up by index, so the later day can never be matched. Latvian and Maltese here are the only locales affected, apart from Sinhala, whose Both spellings also match CLDR, if that helps as a second opinion. One deliberate omission: I left Latvian Monday as |
Pull Request Checklist
Thank you for taking the time to improve Arrow! Before submitting your pull request, please check all appropriate boxes:
toxormake testto find out!).tox -e lintormake lintto find out!).masterbranch.If you have any questions about your code changes or any of the points above, please submit your questions along with the pull request and we will try our best to help!
Description of Changes
This is the locale data half of what I mentioned in #1379. Two locales give the
same
day_abbreviationto two different weekdays:parser.pyresolves thedddtoken with[x.lower() for x in self.locale.day_abbreviations].index(value), andlist.indexreturns the first match, so the second day of each pair isunreachable. Formatting a weekday and parsing it straight back does not round
trip:
The change
For Latvian, the abbreviations are the first two letters of each name, and
piektdiena collides with pirmdiena. Latvian's own short forms break the tie
at the "k" — CLDR uses
P O T C Pk S Svfor the narrow day names — sopiektdiena becomes
"pk". That is the same reasoning the locale alreadyapplies to svētdiena, which takes
"sv"rather than colliding withsestdiena's
"se".For Maltese, Il-Ħamis and Il-Ħadd are identical for their first two
letters, so one extra letter is not enough and they become
"Ħam"and"Ħad". CLDR's abbreviated Maltese forms separate them at the same point,Il-ĦamandIl-Ħad. The locale already mixes abbreviation lengths forexactly this reason, with
"TL"for It-Tlieta against"T"for It-Tnejn,so this follows what is there rather than introducing a new convention.
I kept the change to the two colliding entries. Restyling Maltese to CLDR's
full three-letter set (
Tne Tli Erb Ħam Ġim Sib Ħad) would be more uniform,but it touches days that are not broken, so I would rather do that separately
if you want it at all.
Worth flagging one behavioural consequence:
"Ħ"on its own is no longer aMaltese day abbreviation, so anything that stored that string will no longer
parse. It could not be parsed as Sunday before anyway, and it was ambiguous by
construction, so I think that is the right trade — but it is a change, not
purely additive.
Verification
The round trip above is the test: for each of the fourteen weekdays across the
two locales,
format("ddd")thenget(..., "ddd"). Before the change two ofthe fourteen come back as the wrong day; after it all fourteen are correct.
The new
test_day_abbreviationsin each locale's test class pins every weekdayand also asserts the seven abbreviations are distinct, so a future edit that
reintroduces a collision fails rather than silently making a day unparseable.
Both tests fail on
master:pytest tests/goes from 1901 to 1903 passing, 1 skipped, coverage 99.93%,nothing failing either before or after. The existing
test_weekdayfor bothlocales checks Saturday, which neither change touches.
This is independent of #1379 and touches a different file, so the two do not
conflict and either can land first. #1379 does not fix these two and this does
not fix what #1379 fixes. The third collision I found, Sinhala's
"බදා"forboth Tuesday and Wednesday, is in #1336 and I have left it alone here.
On the two unchecked boxes: I could not run
make lint, because it drivespre-commitand neither that norisortinstalls in this environment. I ranblack --checkandflake8, both clean, and the change touches no imports. Nodocumentation change seemed needed for a locale data correction, but say the
word if you would like a changelog entry.