elixir-cldr / cldr_units

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

`Cldr.Unit.round/2` fails when the unit has float a value #37

Closed doughsay closed 1 year ago

doughsay commented 1 year ago

When upgrading from 3.15.0 to 3.16.1, this code started to fail when it previously didn't:

iex> Cldr.Unit.new!(:meter, "12.34") |> Cldr.Unit.to_float_unit() |> Cldr.Unit.round(1)
# ** (FunctionClauseError) no function clause matching in Cldr.Unit.Conversion.maybe_integer/1

#     The following arguments were given to Cldr.Unit.Conversion.maybe_integer/1:

#         # 1
#         12.3

#     Attempted function clauses (showing 2 out of 2):

#         def maybe_integer(%Decimal{} = a)
#         def maybe_integer(a) when is_integer(a)

#     (ex_cldr_units 3.16.1) lib/cldr/unit/conversion.ex:481: Cldr.Unit.Conversion.maybe_integer/1
#     (ex_cldr_units 3.16.1) lib/cldr/unit/math.ex:365: Cldr.Unit.Math.round/3
#     iex:4: (file)
kipcole9 commented 1 year ago

I published ex_cldr_units version 3.16.2 with the following changelog entry:

Bug Fixes

Thanks much for the report and the PR, really appreciated.

BTW, I would probably change the order of the functions to maximise the precision:

iex> Cldr.Unit.new!(:meter, "12.34") |> Cldr.Unit.round(1) |> Cldr.Unit.to_float_unit() 

I think delaying the call to Cldr.Unit.to_float_unit() to as late as possible is optimal (but not essential in this case).

doughsay commented 1 year ago

Thanks so much for the quick turnaround! And thanks for the suggestion, will definitely keep it in mind. Really I'd like to stop using floats altogether, but that's going to require a bigger refactor 😅