elixir-cldr/cldr_units

Missing/misconfigured units after upgrade

Opened this issue · 7 comments

gf3 commented

Strange issue and I'm not quite sure where it belongs, but it seems that after the last update of ex_cldr and ex_cldr_units some units have gone missing or become misconfigured. I've pored through the changelogs of ex_cldr_units, ex_cldr, and the actual CLDR 45 release and I haven't found anything that sticks out to me.

It seems that units that are a division or multiplication of :meter are misbehaving (e.g. :millimeter and :kilometer). Possibly this has happened to other units as well and I simply haven't noticed yet.

Example

Before:

MyApp.Cldr.Unit.display_name(:millimeter, style: :long) #=> "millimeters"

After:

MyApp.Cldr.Unit.display_name(:millimeter, style: :long) #=> {:error, {Cldr.Unit.UnitNotTranslatableError, "The unit :millimeter is not translatable"}}

Reproduce

Below is a script to reproduce the issue described above. Things to note:

  1. :millimeter is missing from MyApp.Cldr.Unit.know_units
  2. :millimeter is present in Cldr.Unit.know_units_for_category(:length)
Mix.install([
  {:jason, "~> 1.0"},
  {:ex_cldr, "2.38.1"},
  {:ex_cldr_units, "3.17.0"}
])

defmodule MyApp.Cldr do
  use Cldr,
    locales: ["en",],
    default_locale: "en",
    providers: [Cldr.Number, Cldr.Unit, Cldr.List]
end

IO.puts("known units")
dbg(Enum.sort(MyApp.Cldr.Unit.known_units))

IO.puts("known units contains: millimeters")
dbg(Enum.find(MyApp.Cldr.Unit.known_units, fn unit -> unit == :millimeter end))

IO.puts("units for category: length")
dbg(Cldr.Unit.known_units_for_category(:length))

IO.puts("display name: millimeter")
dbg(MyApp.Cldr.Unit.display_name(:millimeter, style: :long))

Note that you can reproduce the successful output by reverting the dependencies at the beginning of the script to:

Mix.install([
  {:jason, "~> 1.0"},
  {:ex_cldr, "2.37.5"},
  {:ex_cldr_units, "3.16.5"}
])

Sorry for the slow reply.

While there isn't much change in CLDR 45 on the units side, there is quite a bit of change on this library because I was my implementation was making some invalid assumptions. I was correlating translatable units with known units and thats not how CLDR intends. That meant a lot of work under the covers, and all the tests from previous releases pass.

But clearly I have more work to do and I will work on this today and keep this thread updated.

Part of the changes in implementation do relate to prefixes (SI and others). I think it's likely that Cldr.Unit.known_units/0 will only return base units - with prefixes being valid on any unit. I'll review that today as well.

On first review it looks like the bug is in determining what is a translatable unit so hopefully not too hard to pin down.

gf3 commented

Thank you for the detailed reply, @kipcole9. Please let me know if I can assist in any way.

@kipcole9 Any updates on this one? Anything we can help you with? Also been relying on display_name to shorten + localize unit names, and keeping ex_cldr_units below 3.17 keeps us from upgrading the rest of your libraries now.

I think we're seeing the same thing, can you confirm if this is the same issue?

iex(15)> Cldr.Unit.new!(:kilogram, 1.1)
Cldr.Unit.new!(:kilogram, "1.1")
iex(16)> Cldr.Unit.new!(:kilometer, 1.1)
Cldr.Unit.new!("kilometer", "1.1")

It seems like some units will remain atoms after new!/1 is called, but some units are reset as strings.

I've not been covering myself in glory on this - and it's my favourite cldr lib too. I made some fundamental errors in understanding the spec when I implemented it, and that's come back to bite me in the last few releases. I have done a lot of work to get things back on track, but not completed.

CLDR 47 is due out in October and I've done preliminary integration and everything is looking ok. Which means I can now get back to this library and get it properlly sorted out. The main challenge now is that I'm travelling with limited internet access. Realistically it means I can't complete the work and update the lib until mid November.

All of the issues noted in this thread (excepting the Math module which needs serious work and will be after fixing this issue) related to that original design error. The API will remain the same - only the underlying unit struct will change a little bit such that all the units and unit parts will be strings. And the process of resolving translations and validating units will change quite a bit to accommodate the additional unit prefixes and the revised localisation process.

I know this issues is dragging on. I will get this sorted and on a more robust foundation for the November release.

gf3 commented

Thanks for the update @kipcole9. Looking forward to it—please let us know if we can help when it comes to test/release time. Enjoy your travels!