elixir-cldr / cldr_units

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

(FunctionClauseError) no function clause matching in Cldr.Unit.Parser.extract_base_unit/1 #12

Closed ghost closed 4 years ago

ghost commented 4 years ago

👋

Version:

* ex_cldr_units 3.0.0 (Hex package) (mix)
  locked at 3.0.0 (ex_cldr_units) 6b0c3db6

Seems like extract_base_unit/1 doesn't expect composite units like :kilowatt_hour since they seem to have several base_units?

iex(3)> Demo.Cldr.Unit.to_string(1, unit: :kilowatt_hour)
** (FunctionClauseError) no function clause matching in Cldr.Unit.Parser.extract_base_unit/1

    The following arguments were given to Cldr.Unit.Parser.extract_base_unit/1:

        # 1
        {:kilowatt_hour,
         [
           hour: %Cldr.Unit.Conversion{base_unit: [:second], factor: 3600, offset: 0},
           kilowatt: %Cldr.Unit.Conversion{
             base_unit: [:kilogram_square_meter_per_cubic_second],
             factor: 1000,
             offset: 0
           }
         ]}

    Attempted function clauses (showing 2 out of 2):

        defp extract_base_unit({_unit_name, [{_, %{base_unit: base_units}}]})
        defp extract_base_unit({_unit_name, %{base_unit: base_units}})

    (ex_cldr_units 3.0.0) lib/cldr/unit/parser.ex:307: Cldr.Unit.Parser.extract_base_unit/1
    (ex_cldr_units 3.0.0) lib/cldr/unit/parser.ex:226: Cldr.Unit.Parser.canonical_base_unit/1
    (ex_cldr_units 3.0.0) lib/cldr/unit.ex:1061: Cldr.Unit.unit_category/1
    (ex_cldr_units 3.0.0) lib/cldr/unit.ex:220: Cldr.Unit.validate_usage/2
    (ex_cldr_units 3.0.0) lib/cldr/unit.ex:206: Cldr.Unit.create_unit/3
    (ex_cldr_units 3.0.0) lib/cldr/unit.ex:441: Cldr.Unit.to_string/3
kipcole9 commented 4 years ago

Use a string unit. Atom units are assumed to be directly translatable (ie CLDR provides a translation for this unit). String units are compose of multiple units. It should be more resilient than this for sure. Try this:

iex> Demo.Cldr.Unit.to_string(1, unit: "kilowatt hour")
{:ok, "1 hourâ‹…kilowatt"}

Note that under the covers there is an algorithm that orders base units in a canonical form so the output here I appreciate isn't exactly how you would say it in English at least.

kipcole9 commented 4 years ago

Cldr.Unit.known_units/0 will give you a list of the units that are directly translatable from CLDR data. All other units are composite and hence the translations are also composed from translatable sub components.

kipcole9 commented 4 years ago

So now I have a few things to look at for the weekend (which I am fine to do)

  1. Don't use canonical ordering for unit strings - use the original ordering. The requires a bit of work since I will need to keep the unit ordering in the state. And I'm not sure this works for all languages. That is, is kilowatt hour expressed as Kilowattstunde in German? Can I assume the same word order in all languages? I don't know.

    iex> MyApp.Cldr.Unit.to_string(1, unit: "kilowatt_hour", locale: "de")
    {:ok, "1 Stundeâ‹…Kilowatt"}

    Isn't very friendly for German speakers, but thats how the data guides the format to become.

  2. Provide a way to localise the unit name separately from the unit value. That part isn't too difficult, I'll make that happen. This addresses you plan to decorate them differently.

  3. Be more resilient if an atom unit name isn't directly translatable, use the string form and see if that parses.

  4. Write to the CLDR users mailing list and see how other implementers are managing this.

kipcole9 commented 4 years ago

Ohhhh, this is also a bug since :kilowatt_hour is directly translatable. I need to find out why its not being treated that way.

kipcole9 commented 4 years ago

Fixed this bug in this commit and published on hex as ex_cldr_units version 3.0.1.

You should now be able to use :kilowatt_hour and "kilowatt hour" interchangeably.

The translations should now also be correct since this is a directly translatable unit:

iex> Cldr.Unit.new!(1, :kilowatt_hour) |> MyApp.Cldr.Unit.to_string              
{:ok, "1 kilowatt hour"}

iex> Cldr.Unit.new!(1, :kilowatt_hour) |> MyApp.Cldr.Unit.to_string(locale: "de")
{:ok, "1 Kilowattstunde"}

iex> Cldr.Unit.new!(1, "kilowatt_hour") |> MyApp.Cldr.Unit.to_string             
{:ok, "1 kilowatt hour"}

iex> Cldr.Unit.new!(1, "kilowatt_hour") |> MyApp.Cldr.Unit.to_string(locale: "de")
{:ok, "1 Kilowattstunde"}

Leaving the other issue open for work this weekend.

ghost commented 4 years ago

@kipcole9 Thank you!