search

elixir-cldr / cldr_units

Unit formatting (volume, area, length, ...) functions for the Common Locale Data Repository (CLDR)
Other
16 stars 13 forks source link

Regressions from 3.16 #45

Open Awlexus opened 1 month ago

Awlexus commented 1 month ago

Hi, we ran into a couple of issues updating this library from 3.16.4 to 3.17

1. Cldr.Unit.parse_unit_name returns string instead of atom

iex> Cldr.Unit.parse_unit_name("mg/dl")
# Before 3.17
{:ok, :milligram_ofglucose_per_deciliter}
# After 3.17
{:ok, "milligram_ofglucose_per_deciliter"}

2. Cannot format Unit with unit_name "mg/dl"

The following function call does not work in 3.17 any more

iex> Cldr.Unit.to_string!(Cldr.Unit.new!("milligram_ofglucose_per_deciliter", "120.5"), style: :short)
# Before 3.17
"120.5 mg/dL"
# After 3.17
** (Cldr.Unit.NoPatternError) No format pattern was found for unit Cldr.Unit.new!("milligram_ofglucose_per_deciliter", "120.5") with grammatical case :nominative, gender :neuter and plural type :other
     stacktrace:
       (ex_cldr_units 3.17.0) lib/cldr/unit/format.ex:807: Cldr.Unit.Format.get_unit_pattern!/3
       (ex_cldr_units 3.17.0) lib/cldr/unit/format.ex:714: Cldr.Unit.Format.do_iolist/3
       (ex_cldr_units 3.17.0) lib/cldr/unit/format.ex:682: Cldr.Unit.Format.do_iolist/3
       (ex_cldr_units 3.17.0) lib/cldr/unit/format.ex:631: Cldr.Unit.Format.to_iolist/4
       (ex_cldr_units 3.17.0) lib/cldr/unit/format.ex:639: Cldr.Unit.Format.to_iolist/4
       (ex_cldr_units 3.17.0) lib/cldr/unit/format.ex:470: Cldr.Unit.Format.to_iolist/3
       (ex_cldr_units 3.17.0) lib/cldr/unit/format.ex:201: Cldr.Unit.Format.to_string/3
       (ex_cldr_units 3.17.0) lib/cldr/unit/format.ex:316: Cldr.Unit.Format.to_string!/3
kipcole9 commented 1 month ago

@Awlexus, sorry for the very late reply - and thank you for the report. Clearly a poor regression which I'll work on.

I am, overall, not making fast enough progress on this library. In large part because with the release of CLDR45 I released that I have not correctly understood the specification for units of measure and my implementation isn't correct. I am increasingly of the mind that I need to reimplement it from scratch (with basically the same public API). I will finalise the decision to do that - or not - in the next week.

I will work on fixing this bug right away.

In the current implementation, unit names can be either an atom (which indicates there is a direct localisation available) or a string (which needs to be processed to localise). This is part of the core misunderstanding on my part of the spec.