elixir-cldr / cldr_dates_times

Date & times formatting functions for the Common Locale Data Repository (CLDR) package https://github.com/elixir-cldr/cldr
Other
69 stars 13 forks source link

"fr" time interval appears with "en" time #35

Closed Gladear closed 1 year ago

Gladear commented 1 year ago

Minimal reproduction

I've got a Cldr module which defines French within it's supported locales:

defmodule TestCldrFrInterval.Cldr do
  use Cldr,
    locales: ["fr"],
    providers: [Cldr.Number, Cldr.Calendar, Cldr.DateTime]
end

and this calling module and function

defmodule TestCldrFrInterval do
  def hello do
    Cldr.put_locale("fr")
    TestCldrFrInterval.Cldr.Time.to_string!(~T[12:00:00]) |> IO.inspect(label: "time")
    TestCldrFrInterval.Cldr.Time.Interval.to_string!(~T[12:00:00], ~T[13:00:00]) |> IO.inspect(label: "interval")
  end
end

with config/config.exs

import Config

config :ex_cldr, default_backend: TestCldrFrInterval.Cldr

Expected

I expected the output to be along the lines of

time: "12:00:00"
interval: "12:00:00 – 13:00:00"

Got

time: "12:00:00"
interval: "12:00 – 1:00 PM"

Note that Date.Interval and DateTime.Interval return the expected output, in french locale.

kipcole9 commented 1 year ago

@Gladear thank you for the report, and sorry for the inconvenience. This could be an error in the data, but more likely an bug in my code. Its surprisingly complex to derive the right time format and I'm probably not quite getting it right. I'm travelling the next few days but will do my best to resolve this by the end of the weekend. Hope that's ok.

Gladear commented 1 year ago

Thanks for the quick answer ! I would consider it a really fast fix if it is fixed by the end of the week-end 😄 Don't rush, everything is fine 🙂

kipcole9 commented 1 year ago

I set my self a goal to resolve issues within 4 hours. I'm definitely not meeting that goal in the last 30 days, but I will get back to it in April! (thanks for the patience!).

kipcole9 commented 1 year ago

TLDR; Looks like a data problem and I will open a case on the CLDR Jira. But sometimes its not a data issue but a spec interpretation issue.

Summary

To me this looks like a data error in CLDR. But its also possible I'm misreading the spec.

Time formats seems as expected

If we look at the available time formats for the :fr locale we see the following:

iex> MyApp.Cldr.DateTime.Format.time_formats :fr                                                                                   {:ok,
 %Cldr.Time.Styles{
   short: "HH:mm",
   medium: "HH:mm:ss",
   long: "HH:mm:ss z",
   full: "HH:mm:ss zzzz"
 }}

Where the format code H means "24 hour time". So far so good and its why you see time formats (and Date Time formats) being formatted as expected.

Interval formats seem incorrect for the hour unit

Now lets look at the interval formats for :fr. Although there is a lot of data, the key principle is that the code determines what the required scale and precision required is (year, month, day, hour, minute, second) and what the unit with the greatest difference is between the from and to parts of the interval and it makes a selection (year, that wasn't a fun part to write).

Basically for time formats you can look at the :hm and :hmv formats since they are used most commonly for formatting time intervals. You will note the formats use h (lower case h) not H. And therefore the format explicitly says use 12-hour times. Which is not expected.

iex> MyApp.Cldr.DateTime.Format.date_time_interval_formats :fr                                                                     
{:ok,
 %{
   bh: %{b: ["h B – ", "h B"], h: ["h – ", "h B"]},
   bhm: %{
     b: ["h:mm B – ", "h:mm B"],
     h: ["h:mm – ", "h:mm B"],
     m: ["h:mm – ", "h:mm B"]
   },
   d: %{d: ["d–", "d"]},
   gy: %{g: ["y G 'à' ", "y G"], y: ["y–", "y G"]},
   gy_m: %{
     g: ["MM/y G – ", "MM/y G"],
     m: ["MM–", "MM/y G"],
     y: ["MM/y – ", "MM/y G"]
   },
   gy_m_ed: %{
     d: ["E d – ", "E d/MM/y G"],
     g: ["E d/MM/y G – ", "E d/MM/y G"],
     m: ["E d/MM – ", "E d/MM/y G"],
     y: ["E d/MM/y – ", "E d/MM/y G"]
   },
   gy_md: %{
     d: ["d–", "d/MM/y G"],
     g: ["d/MM/y G – ", "d/MM/y G"],
     m: ["d/MM – ", "d/MM/y G"],
     y: ["d/MM/y – ", "d/MM/y G"]
   },
   gy_mm_md: %{
     d: ["d–", "d MMM y G"],
     g: ["d MMM y G – ", "d MMM y G"],
     m: ["d MMM – ", "d MMM y G"],
     y: ["d MMM y – ", "d MMM y G"]
   },
   gy_mmm: %{
     g: ["MMM y G – ", "MMM y G"],
     m: ["MMM – ", "MMM y G"],
     y: ["MMM y – ", "MMM y G"]
   },
   gy_mmm_ed: %{
     d: ["E d – ", "E d MMM y G"],
     g: ["E d MMM y G – ", "E d MMM y G"],
     m: ["E d MMM – ", "E d MMM y G"],
     y: ["E d MMM y – ", "E d MMM y G"]
   },
   h: %{h: ["HH – ", "HH"]},
   hm: %{
     a: ["h:mm a – ", "h:mm a"],
     h: ["h:mm – ", "h:mm a"],
     m: ["h:mm – ", "h:mm a"]
   },
   hmv: %{
     a: ["h:mm a – ", "h:mm a v"],
     h: ["h:mm – ", "h:mm a v"],
     m: ["h:mm – ", "h:mm a v"]
   },
   hv: %{a: ["h a – ", "h a v"], h: ["h – ", "h a v"]},
   m: %{m: ["M–", "M"]},
   m_ed: %{
     d: ["E dd/MM – ", "E dd/MM"],
     m: ["E dd/MM – ", "E dd/MM"]
   },
   md: %{d: ["dd/MM – ", "dd/MM"], m: ["dd/MM – ", "dd/MM"]},
   mm_md: %{d: ["d–", "d MMM"], m: ["d MMM – ", "d MMM"]},
   mmm: %{m: ["MMM–", "MMM"]},
   mmm_ed: %{d: ["E d – ", "E d MMM"], m: ["E d MMM – ", "E d MMM"]},
   y: %{y: ["y–", "y"]},
   y_m: %{m: ["MM/y – ", "MM/y"], y: ["MM/y – ", "MM/y"]},
   y_m_ed: %{
     d: ["E dd/MM/y – ", "E dd/MM/y"],
     m: ["E dd/MM/y – ", "E dd/MM/y"],
     y: ["E dd/MM/y – ", "E dd/MM/y"]
   },
   y_md: %{
     d: ["dd/MM/y – ", "dd/MM/y"],
     m: ["dd/MM/y – ", "dd/MM/y"],
     y: ["dd/MM/y – ", "dd/MM/y"]
   },
   y_mm_md: %{
     d: ["d–", "d MMM y"],
     m: ["d MMM – ", "d MMM y"],
     y: ["d MMM y – ", "d MMM y"]
   },
   y_mmm: %{m: ["MMM–", "MMM y"], y: ["MMM y – ", "MMM y"]},
   y_mmm_ed: %{
     d: ["E d – ", "E d MMM y"],
     m: ["E d MMM – ", "E d MMM y"],
     y: ["E d MMM y – ", "E d MMM y"]
   },
   y_mmmm: %{m: ["MMMM – ", "MMMM y"], y: ["MMMM y – ", "MMMM y"]}
 }}
kipcole9 commented 1 year ago

For now, if you update to ex_cldr_dates_times version 2.13.2 you can specify the format yourself. Its not as elegant as you should expect but it might move you forward for now:

Example using a string format

iex> MyApp.Cldr.Time.Interval.to_string(~T[12:00:00], ~T[13:00:00], format: "HH:mm:ss - HH:mm:ss", locale: :fr)
{:ok, "12:00:00 - 13:00:00"}
kipcole9 commented 1 year ago

It's my problem. In the original CLDR data (in XML) the right formats exist.

The fix this I need to fix the data ingestion process in ex_cldr and do some work to hook up Cldr.Time. hour_format_from_locale/1 to the format selection process (ie pick either :hm or :Hm).

The error happens because the current ingestion process downcasts the format names and thereby :Hm is overwritten by :hm.

Fixing this will take a few days. And CLDR 43 is also less than a month away so I need to work out if this remediation is best done for a maintenance release or align with with CLDR 43. I'll figure that all out by tomorrow.

kipcole9 commented 1 year ago

I've published ex_cldr_dates_times 2.13.3 with the following changelog entry:

This release requires (and configures) ex_cldr version 2.36.0 or later. That version fixes the interval format data so that formats required by locales that use 24-hour times are not overwritten by data for 12-hour formats. As a result, formatting intervals using a format key directly (ie one of the keys in the map returned by MyApp.Cldr.DateTime.Format.date_time_interval_formats/1) may find the key has changed.

Bug Fixes

Of course please re-open the issue if required.

Gladear commented 1 year ago

That was so fast ! Thanks a lot for your fix 🙂 Have a nice day !