Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
drepamig opened this issue Sep 5, 2024 · 6 comments

Comments

@drepamig
Copy link

drepamig commented Sep 5, 2024

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!

@drepamig drepamig added the bug label Sep 5, 2024
@PesBandi
Copy link
Contributor

PesBandi commented Sep 8, 2024

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
Copy link
Author

drepamig commented Sep 9, 2024

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
Copy link
Owner

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
Copy link
Author

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!

Copy link

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.

Copy link

github-actions bot commented Nov 2, 2024

This issue was automatically closed due to inactivity.

@github-actions github-actions bot closed this as completed Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants