A :duration vs :year_duration inconsistency

DaTrader commented 2 years ago

There's an inconsistency between the known unit categories and the unit category I get for a duration in years as show below:

iex(7)> Cldr.Unit.known_unit_categories()
[:acceleration, :angle, :area, :concentr, :consumption, :digital, :duration,
 :electric, :energy, :force, :frequency, :graphics, :length, :light, :mass,
 :power, :pressure, :speed, :temperature, :torque, :volume]
iex(8)> per = Cldr.Unit.parse!( "2y")
#Cldr.Unit<:year, 2>
iex(9)> { :ok, :duration} = Cldr.Unit.unit_category( per)
** (MatchError) no match of right hand side value: {:ok, :year_duration}

If this is returning :year_duration is the desired behavior here, how do I get to know all possible return values for my duration units so I can match against them?

kipcole9 commented 2 years ago

This one is a little trickier, definitely a corner case. Will take a few hours to resolve properly.

kipcole9 commented 2 years ago

Fixed on master. Following on from the discussion on ex_cldr_calendars, here is my recommendation on formatting days, weeks and quarters:

Days and Weeks (and Months, Years, Decades and Centuries)

These are all built-in units so no special handling is required.

iex> days = Cldr.Unit.new!(:day, 2)
#Cldr.Unit<:day, 2>
iex> MyApp.Cldr.Unit.to_string days
{:ok, "2 days"}

iex> weeks = Cldr.Unit.new!(:week, 2)
#Cldr.Unit<:week, 2>
iex> MyApp.Cldr.Unit.to_string weeks 
{:ok, "2 weeks"}
iex> MyApp.Cldr.Unit.to_string weeks, locale: "fr"
{:ok, "2 semaines"}
iex> MyApp.Cldr.Unit.to_string weeks, locale: "de"
{:ok, "2 Wochen"}
iex> MyApp.Cldr.Unit.to_string weeks, locale: "he"
{:ok, "שבועיים"}

Quarters

The unit quarter is not defined by CLDR but I have opened a ticket. In the mean time you can define a custom unit. Apply the following:

Define the :quarter unit in config.exs:

config :ex_cldr_units, :additional_units,
quarter: [
base_unit: :year,
factor: %{numerator: 1, denominator: 4},
offset: 0,
sort_before: :all,
]

Define the localisations in your backend module (note the addition of use Cldr.Unit.Additonal at the top of the module):

defmodule MyApp.Cldr do
use Cldr.Unit.Additional

use Cldr,
locales: ["en", "fr"],
default_locale: "en",
providers: [Cldr.Number, Cldr.Unit, Cldr.List]

# Unit Quarter

unit_localization(:quarter, "en", :long,
nominative: %{
  one: "{0} quarter",
  other: "{0} quarters"
},
display_name: "quarters"
)

unit_localization(:quarter, "en", :short,
nominative: %{
  one: "{0} qtr",
  other: "{0} qtrs"
},
display_name: "quarters"
)

unit_localization(:quarter, "en", :narrow,
nominative: %{
  one: "{0} q",
  other: "{0} q"
},
display_name: "q"
)

unit_localization(:quarter, "fr", :long,
nominative: %{
  one: "{0} quartier",
  other: "{0} quartiers"
},
display_name: "quartiers"
)

unit_localization(:quarter, "fr", :short,
nominative: %{
  one: "{0} qtr",
  other: "{0} qtrs"
},
display_name: "quartiers"
)

unit_localization(:quarter, "fr", :narrow,
nominative: %{
  one: "{0} q",
  other: "{0} q"
},
display_name: "q"
)
end

Unit creation and formatting as normal

iex> MyApp.Cldr.Unit.to_string Cldr.Unit.new!(:quarter, 1)
{:ok, "1 quarter"}
iex> MyApp.Cldr.Unit.to_string Cldr.Unit.new!(:quarter, 2)
{:ok, "2 quarters"}
iex> MyApp.Cldr.Unit.to_string Cldr.Unit.new!(:quarter, 2), locale: "fr"                      
{:ok, "2 quartiers"}

kipcole9 commented 2 years ago

I have published ex_cldr_units version 3.9.2 which includes the fix for this issue.

DaTrader commented 2 years ago

Great!

DaTrader commented 2 years ago

Kip,

there's another practical concern I have regarding the narrow Cldr.Unit formats: they can and they do overlap sometimes when parsing and thus we come to another inconsistency i.e. the inability to do a roundtrip such as: unit -> to_string -> parse string -> unit.

I am talking about weeks here e.g. the string output for #Cldr.Unit< :week, 3> is "3w" which is correct, but it requires "3wk" to parse it into weeks and not the watts.

Would it be CLDR compliant to add an optional hint as to which unit category to parse into, eg:

Cldr.Unit.parse!( "3w", category: :duration)

I for one have no need whatsoever to parse watts or most other unit categories, so in my case (as in pretty much most other applications), the one or two unit categories in use are known well upfront. This would also help or even speed up the parsing process so that it spends less time parsing and eventually fails instead of returning false positives given the more limited scope.

My 2c,

Damir

kipcole9 commented 2 years ago

More like 25c I think :-) :-)

I don't think round-trip is necessarily a CLDR design goal, but I think what you ask is possible without compatibility issues. Will think on how best to do this and revert.

DaTrader commented 2 years ago

You may be right. I should account for the central bank induced inflation. 25c it is then :)

DaTrader commented 2 years ago

Another thing, what do you suggest how to go about encoding/decoding the units? Make my own Ecto.Type or simply encode them as JSON arrays of two? Any library already available for this?

kipcole9 commented 2 years ago

ex_cldr_units_sql should do the trick.

DaTrader commented 2 years ago

Thanks. Maybe it'd be useful to have this library added to the list on the main ex_cldr documentation page?

DaTrader commented 2 years ago

Ah, I now see you it in the bottom of the ex_cldr_units page. My bad.

kipcole9 commented 2 years ago

I have published ex_cldr_units version 3.10.0 with the following changelog entry. You can see some examples of the :only and :except usage in the tests and the documentation for Cldr.Unit.parse/2.

Bug Fixes

Further refinement to Cldr.Unit.unit_category/1 to return a result in a broader range of cases.

Enhancements

Adds :only and :except options to Cldr.Unit.parse/2. These options provide a mechanism to disambiguate the unit when a unit string could refer to more than one unit. For example, "2w" could refer to either "2 weeks" or "2 watts". If neither option is provided then the result is the same as in prior releases: the unit with the lexically shorter and alphabetically earlier unit is returned.

DaTrader commented 2 years ago

Yes!!! Thanks!

DaTrader commented 2 years ago

Kip,

just to let you know that upgrading ex_units from 3.9.2 to 3.10.0 results in the following:

iex(4)> Cldr.Unit.parse( "52m", only: :duration)
{:error,
 {Cldr.Unit.AmbiguousUnitError,
  "The string \"m\" ambiguously resolves to [:month, :minute]"}}

Minutes should be probably left as min, and not m :)

My 25c (the inflation!),

Damir

kipcole9 commented 2 years ago

hmmm, tricky. in the CLDR data both minute and month have a "narrow" string of m. And I really don't want to "special case" the data - that's like to be unmaintainable and also drive me mad. Other than saying to the user "keep typing ..." I'm not sure what else to do.

DaTrader commented 2 years ago

First, Happy New Year, Kip!

hmmm, tricky. in the CLDR data both minute and month have a "narrow" string of m. And I really don't want to "special case" the data - that's like to be unmaintainable and also drive me mad. Other than saying to the user "keep typing ..." I'm not sure what else to do.

In that case, the only reasonable option IMO is to allow for expanding on the hint specification i.e.:

Cldr.Unit.parse( "52m", only: [ :day, :week, :month, :year]) This would be my parsing case here for I am only interested in the day+ durations. Also worth noting that in this particular case I don't even support the :quarter duration for a reason (unlike in reporting), so specifying a list of desirable outcomes is the most convenient way to go.

kipcole9 commented 2 years ago

Will add this over the next couple of days, sorry for the delay. I want to provide an option for compile-time validation of the units/categories as well as the current runtime variety.

DaTrader commented 2 years ago

No biggie. I am still on 3.9.2 and there it works with just "m" for months.

kipcole9 commented 2 years ago

I have published ex_cldr_units version 3.11.0 with the following changelog:

Bug Fixes

Fix canonical base unit calculation when the unit is a per per form like candela per lux.

Enhancements

Add unit filters for Cldr.Unit.parse/2. This means that the options :only and :except can comprise both unit categories and unit names as part of the filter.

Note that the filters are validated at runtime, not compile time. If this becomes a performance issue then I will consider alternative approaches. I think validating the filter arguments is important since otherwise it could lead to hard-to-debug use cases.

kipcole9 commented 2 years ago

Thanks. Maybe it'd be useful to have this library added to the list on the main ex_cldr documentation page?

Its listed here, is there some other place you think it should be listed?

DaTrader commented 2 years ago

No, it's ok. I saw it later in the bottom of the ex_cldr_units page.

elixir-cldr / cldr_units