How to handle ambiguous unit abbreviations

[angularsen]

There are several units with ambiguous unit abbreviations, and there is currently no good way to handle this. This issues is created as a place to discuss solutions.

Example: #131
Metric cup, US legal cup and customary cups might be natural to assign "cup" or "metric cup" as abbreviations, but "cup" is ambiguous and "metric cup" is currently not supported due to whitespace.

[neutmute]

Is the use case for .Parse(string input) to be able to take natural language input and turn it into a quantifiable unit?
If so then users are most likely to just type "1 cup" - rather than "1 metric cup" or "1 legal cup". Which exact cup they mean is driven by context that only the application can know - ie: An Australian would expect "1 cup" to imply "1 metric cup" and may not even be aware of other definitions of cups (I wasn't until today :)). An American nutritionist would perhaps expect "1 cup" to be interpreted as "1 legal cup".

That is to say I don't think the problem is solvable from a parsing perspective. Support for whitespaces could help but I don't think users would know/like needing to type "1 metric cup".

Idea: Optionally allow the application to supply configurable mappings to the Parse method.
eg: "cup" => VolumeUnit.MetricCup.
These would override / supplement the built in abbreviations.

re: ToString(): In the absence of an abreviation it could fall back to using the enum unit for in these cases? ie: $"{Quantity} {EnumUnit.ToString()";
Useful for debug/watch windows and logs but not so much for UI output for an end user. Again in that case I think the application could just 'know the context' and emit the appropriate text instead.

[angularsen]

Agree, there is probably no way for Parse() to handle ambiguous units. I don't even know how the current implementation handles this, but I imagine it chooses an arbitrary one (the one defined last in json, or possibly the last in lexical ordering?)

1. I propose Parse() throws an exception if there are ambiguous units defined, it's better than getting a calculation error due to two very similar units. I can imagine most programmers are not aware of this problem in most cases, such as the cups already mentioned and metric ton vs long and short tons.
2. I propose units that naturally wants to share the same abbreviation should do so. In the cups example, this means ToString() will output "1 cup" for all of them, but Volume.Parse("1 cup") will throw an exception due to the ambiguity.

[angularsen]

I don't think we should fall back to EnumUnit.ToString() if no abbreviations are defined. I'd rather keep it simple and require it to be defined in JSON, and simply use no abbreviation if none is defined.

Or else we will get trouble with units like RatioUnit.DecimalFraction, it has no unit abbreviation and none should be output either.

[neutmute]

Good points - agree on all

[neutmute]

If you think that is all that needs covering, happy to do a PR that

1. Adds cup abbreviations to Metric/Us/Customary cup in JSON
2. Ensure Parse template throws an exception when ambiguous

Should the exception be a new AmbiguousTypeException say? Or something generic: NotSupportedException?

[angularsen]

Awesome.
I'd suggest adding a UnitsNet specific exception, such as UnitsNet.AmbiguousUnitParseException, which tells me it's due to an ambiguous unit in when parsing. This allows us to handle this specific case if need be.

As for when to throw, iirc you'll have to dig into the UnitSystem class and look into the dictionaries it uses to organize unit enums and abbreviation strings for lookup.
It's important that it throws when what you are specifying is ambiguous, such as Volume.Parse("1 cup"), but it should not throw on Volume.Parse("1 L"), since it is not ambiguous even if it is the same unit class. I would want unit tests to confirm at least these two cases.

[angularsen]

And, I believe there is a UnitsNetException already, derive from that so there is a way to catch all UnitsNet related exceptions by its base exception type.

[neutmute]

👍

[neutmute]

I had a stab at this but it is trickier than I thought.

The UnitSystem.MapUnitToAbbreviation precaches all the mappings before a parse is attempted.This method is where the duplicate abbreviation must be detected but it cannot throw here as that unit may never be parsed.

So what I did was instead of ignoring a duplicate, flag that abbrievation as bad by poisoning the dictionary with a -1.
Works ok at first glance - I can throw an exception where expected - except it has a side effect of a strange unit test bug where Parse_ReturnsUnitMappedByCustomAbbreviationOrUndefined sometimes fails:

• It won't fail when test run individually.
• It will fail if run in a suite of tests from that assembly

Experimental code here @ neutmute@8b2a12c - it isn't production quality and I would rebase to use a constant for the -1 flag / proper exception / properly change the template rather than generated coed.

Any thoughts on the strategy/unit test failure? The latter smells like a caching issue in the test but I don't get why it would poison the area unit. Setting a breakpoint on my poisoning line confirms it is only triggered on 'cups'.

[neutmute]

cool - thanks for guidance on neutmute@8b2a12c.
Wasn't sure on etiquette of PR'ing experimental code but will do that next time