angularsen / UnitsNet

Makes life working with units of measurement just a little bit better.
https://www.nuget.org/packages/UnitsNet/
MIT No Attribution
2.65k stars 382 forks source link

Quantity.TryParse won't parse "10 MM" because "MM" is capitalized. Other abbreviations work fine. #1423

Closed drepamig closed 3 weeks ago

drepamig commented 2 months ago

Describe the bug Quantity.TryParse(type, string, out IQuantity value) fails to parse Length strings with capital MM (i.e., "10 MM" should parse just as easily as "10 mm"). Other abbreviations appear to work, regardless of casing.

To Reproduce See .NET Fiddle here: http://dotnetfiddle.net/3JmUsw

Expected behavior The string should be converted to a Length based on a millimeter value

Additional context I believe this used to work correctly, but I'm not certain.

Thanks for this library, it's a massive time saver!

PesBandi commented 2 months ago

Hi @drepamig, IMO this isn't a bug. The symbol for the millimeter is mm, not MM. Unit symbols are case sensitive, therefore parsing MM as millimeters would be incorrect. I understand that many people (incorrectly) use MM for millimeters, however that does not mean UnitsNet should treat it that way. Moreover MM is the symbol for another unit - the megamolar.

drepamig commented 2 months ago

Hi @PesBandi, upon further inspection, I realize that overlap in prefixes milli (m-) and mega (M-) are likely the reason for this behavior. I never use megameters/liters/grams in my industry (or life, to be honest), so I didn't even consider that would be the issue. However, that said, the rest of the unit abbreviations are not case sensitive, except where there is overlap (i.e., milli (m) vs mega (M), quetta (Q) vs. quecto (q) and ronna (R) vs ronto (r)). There may be others, but those are the ones I found on my spot check.

I agree this is not a bug, per se, though the lack of case-sensitivity in the other units caused confusion.

angularsen commented 2 months ago

Hi, yes this is "by design".

There is a quirk that maybe causes the confusion; UnitsNet is actually case insensitive if it can unambiguously identify a unit from the case-insensitive lookup of the unit abbreviation "MM" among all Length units. But, as you point out, "MM" could mean millimeters and megameters when ignoring case, so it falls back to case-sensitive matching and finds no match, since megameters would be "Mm", not "MM".

There are good use cases for allowing case insensitive matching, but I think maybe we could avoid this confusion if the exception described the competing units and maybe a bit more about this behavior?

drepamig commented 2 months ago

Hi, yes this is "by design".

There is a quirk that maybe causes the confusion; UnitsNet is actually case insensitive if it can unambiguously identify a unit from the case-insensitive lookup of the unit abbreviation "MM" among all Length units. But, as you point out, "MM" could mean millimeters and megameters when ignoring case, so it falls back to case-sensitive matching and finds no match, since megameters would be "Mm", not "MM".

There are good use cases for allowing case insensitive matching, but I think maybe we could avoid this confusion if the exception described the competing units and maybe a bit more about this behavior?

I think that would be very helpful. The behavior makes sense now that I understand the root cause, but that cause wasn't immediately apparent.

Thanks again for the great utility!

github-actions[bot] commented 1 month ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] commented 3 weeks ago

This issue was automatically closed due to inactivity.