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

Allow Override of Default Unit Conversion #1313

Open
dustincleveland opened this issue Sep 5, 2023 · 2 comments
Open

Allow Override of Default Unit Conversion #1313

dustincleveland opened this issue Sep 5, 2023 · 2 comments
Labels
bug enhancement pinned Issues that should not be auto-closed due to inactivity.

Comments

@dustincleveland
Copy link

Our industry uses specific conversion ratios between units that are not always the standard. For example (according to my best understanding of what was explained to me), the conversion between pressure units of Pascals and Inches of Water Column is an approximation due to certain environmental factors. Therefore, we may want to adjust that conversion to slightly adjust that approximation.

I would like to be able to set a conversion function to override the default. I tried this:

// Using UnitsNET 5.32.0
using UnitsNet;
using UnitsNet.Units;

UnitsNetSetup.Default.UnitConverter.SetConversionFunction<Pressure>(
    PressureUnit.Pascal,
    PressureUnit.InchOfWaterColumn,
    pressure => new Pressure(999, PressureUnit.InchOfWaterColumn));

UnitsNetSetup.Default.UnitConverter.SetConversionFunction<Pressure>(
    PressureUnit.InchOfWaterColumn,
    PressureUnit.Pascal,
    pressure => new Pressure(999, PressureUnit.Pascal));

var pressure = Pressure.FromPascals(1);

Console.WriteLine(pressure.InchesOfWaterColumn);
Console.WriteLine(pressure.As(PressureUnit.InchOfWaterColumn));

// Expected Result: 999
// Actual Result: ~0.004

However, this does not seem to work because Pressure.TryToUnit is called before attempting to use the unit converter to retrieve the default conversion function. I'm not sure if that is a bug, or working as intended, as it is unclear to me how you would extend the PressureUnit enum such that Pressure.TryToUnit would fail to convert. Therefore, I also wonder how the unitConverter argument would ever be made use of? I feel like I must be missing something very obvious. Is it terrible for performance to check the unit converter for a conversion function before calling Pressure.TryToUnit?

It seems like the alternatives are to fork the code base or to define a custom Pressure unit. The latter option makes it unclear to developers which Pressure unit they should be using in the application, along with the API tradeoffs described in the documentation on custom units. Both options are pretty heavy for a slight tweak to the conversion factor.

@angularsen
Copy link
Owner

I'll need some time to get around to this.
It seems like a bug, that maybe custom conversion functions don't always have precedence over the built-in ones?

Not sure, but if you are able to debug to confirm this and write a unit test that fails, then passes with a fix, then that will help speed things up a lot.

@dustincleveland
Copy link
Author

Okay, I will do that!

@angularsen angularsen added bug pinned Issues that should not be auto-closed due to inactivity. labels Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug enhancement pinned Issues that should not be auto-closed due to inactivity.
Projects
None yet
Development

No branches or pull requests

2 participants