-
Notifications
You must be signed in to change notification settings - Fork 385
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
Overrides of unit abbreviations in non-default cultures causes Quantity.Parse to fail #832
Comments
Excellent description, thank you! Would you be willing to attempt a pull request that adds a unit test that reproduces the bug and a bug fix that passes the test? I might find some time to dig into this later, but it seems like you have pretty much nailed the problem already. |
I'll see if I can find some time this weekend. Will let you know. |
I believe the issue is the quantity parser calling into The |
@tmilnthorp Sounds good. I can create a PR at some point for this one (hopefully next weekend), but if anyone wants to put in a fix sooner, I'm cool with that too :) We have a workaround in Discovery, so this isn't a high priority at the moment. |
I've added PR #833 |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Describe the bug
UnitsNet offers a public method that allows consumers of the NuGet package to override the default abbreviations used for different units.
For example, one can write
UnitAbbreviationsCache.Default.MapUnitToDefaultAbbreviation(typeof(MassFlowUnit), (int)MassFlowUnit.GramPerSecond, CultureInfo.CurrentCulture, "grams/second")
in order to use
"grams/second"
instead of"g/s"
as the displayed abbreviation.However, if the current culture is something other than en-US, some problems arise when attempting to call
UnitsNet.Quantity.Parse(Type quantityType, string quantityString)
after performing the above override.
If we had an override as in the example above, attempting to parse any mass flow
string
that doesn't use the"grams/second"
abbreviation would fail in a non-English culture.For example,
Quantity.Parse(typeof(MassFlow), "10 lb/s")
would fail if the current culture were de-DE.
To Reproduce
(1) Add an override similar to the one in the description, using a culture other than en-US.
(2) Call
Quantity.Parse
in the culture that was used specified in your override, and make this call on a quantitystring
that uses a unit abbreviation other than the one you specified in the override.The parse should fail.
(Bug was found in UnitsNet 4.65.0 but hasn't been fixed to my knowledge.)
Expected behavior
Calls to
Quantity.Parse
that succeed without overrides to unit abbreviations should behave the same way with overrides.Without overrides, UnitsNet falls back to using en-US for non-English cultures (in most cases). The override is causing UnitsNet to ignore the fallback abbreviations.
Screenshots
The problem appears to lie in how the following method in UnitAbbreviationsCache.cs is handled. A return value of
true
is given even when the lookup that ends up getting stored inunitToAbbreviations
may not include any abbreviations other than ones that have been specified in an override. At other points in this file, atrue
return value fromTryGetUnitValueAbbreviationLookup
will cause UnitsNet to use the incomplete lookup and ignore the fallback culture.Additional context
I encountered this bug when overriding unit abbreviations, but it would seem to apply also to cases where one simply adds new abbreviations as well.
The text was updated successfully, but these errors were encountered: