chrisimcevoy / pyoda-time

A better date and time API for Python
https://pyodatime.org/
Apache License 2.0
1 stars 0 forks source link

fix(CultureInfo): default locale name should contain hyphen rather than underscore #157

Closed chrisimcevoy closed 4 months ago

chrisimcevoy commented 4 months ago

While implementing #156, I noticed something was amiss with LocaleDateTime. For my locale (en-GB), I should see the date in the format dd/MM/yyyy, but what I actually saw in the REPL was:

>>> from pyoda_time import LocalDateTime
>>> LocalDateTime(year=1, month=2, day=3)
2/3/0001 12:00:00 AM

Looking into it, I discovered that CultureInfo.current_culture.date_time_format.short_date_pattern was giving me M/d/yyyy instead of dd/MM/yyyy.

This was made more confusing by the fact that if I constructed a CultureInfo manually, it gave me the right short date format:

>>> from pyoda_time._compatibility._culture_info import CultureInfo
>>> CultureInfo("en-GB").date_time_format.short_date_pattern
'dd/MM/yyyy'

After a bit more investigation, I discovered that the root cause lay in CultureInfo._get_default_locale_name(), which was returning en_GB (underscore) rather than en-GB (hyphen). Verified via:

>>> CultureInfo("en_GB").date_time_format.short_date_pattern
'M/d/yyyy'

This in itself was perplexing as I would have expected both "en-GB" and "en_GB" to result in Cultures with the same short date format. I went hunting in dotnet to get some answers... This test passes:

    [Test]
    public void EnGbShortDateFormat()
    {
        Assert.That(CultureInfo.CurrentCulture.DateTimeFormat.ShortDatePattern, Is.EqualTo("dd/MM/yyyy"));
        Assert.That(new CultureInfo("en-GB").DateTimeFormat.ShortDatePattern, Is.EqualTo("dd/MM/yyyy"));
        Assert.That(new CultureInfo("en_GB").DateTimeFormat.ShortDatePattern, Is.EqualTo("M/d/yyyy"));
    }

So the take-aways here are:

Oh, and after patching this up, here is what I get in my REPL:

>>> from pyoda_time import LocalDateTime
>>> LocalDateTime(year=1, month=2, day=3)
03/02/0001 00:00:00
codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.48%. Comparing base (e43afe4) to head (49a48de). Report is 25 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #157 +/- ## ========================================== - Coverage 95.49% 95.48% -0.02% ========================================== Files 241 242 +1 Lines 18972 18992 +20 ========================================== + Hits 18118 18134 +16 - Misses 854 858 +4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.