hgrecco / pint

Operate and manipulate physical quantities in Python
http://pint.readthedocs.org/
Other
2.37k stars 466 forks source link

Add a warning when converting from rpm to Hz #1554

Open andrewgsavage opened 2 years ago

andrewgsavage commented 2 years ago

I think we can add a warning when converting between ang freq and freq that links to the page in the docs. Angular freq and freq both have the same dimensionality so can't use that to distinguish between them. I'm thinking to use some logic looking at the to and from units to decide whether a warning should trigger.

Angular freq units: revolutions_per_minute, rpm, revolutions_per_second, rps or any unit with numerator: revolution, radian, degree (could expand to turn, cycle, circle...) denominator: any time unit

Freq units: Hz

Are there any other units this should trigger on? I'm thinking this logic should trigger for most erroneous conversions without too many false positives

ntouran commented 1 year ago

I have a related issue. I'm trying to define relative, absolute, and differential pressure units (similar to #1066) and want to make sure people cannot convert between them. I can define my own in the units definition file, and would like to be able to convert my custom pressure units e.g. psia to inHg_a (a for absolute) but prevent people from converting from psia to psig.

Is there a way to do this? I think it's basically the same question as above.

UPDATE:

Ok I think I have a thing that works for me. I just defined arbitrary other base units to keep custom other units from converting between each other while still leveraging the other conversions.

In the units def file:

# arbitrary units to support gauge/abs/diff pressure units
abs = [absolute]
gau = [gauge]
del = [delta]

# absolute pressure
[pressure_a] = [force] / [area] * [absolute]
pascal_a = abs * Pa = Pa_a
pound_force_per_square_inch_a = abs * psi = psia = psi_a

# gauge pressure
[pressure_g] = [force] / [area] * [gauge]
pascal_g = gau * Pa = Pa_g
pound_force_per_square_inch_g = gau * psi = psig = psi_g

# differential  pressure
[pressure_d] = [force] / [area] * [delta]
pascal_d = del * Pa = Pa_d
pound_force_per_square_inch_d = del * psi = psid = psi_d

This works nicely. I guess for rpm you could do something like

ang= [angular]

rpm_a = angular* rpm
jules-ch commented 1 year ago

I don't think adding a warning is gonna help. It will worsen the UX, one will have to ignore tha warnings.

I'm eager to see if people need those kind of warnings like numpy can do it with RuntimeWarning that the user need to consciously ignore

andrewgsavage commented 1 year ago

ang= [angular] is a solution but was decided against in the last discussion on the topic.

I'm suggesting a warning because this is a topic that comes up regularly tripping up users (myself included!)

Are there any counter examples where you want rpm -> Hz to return 2pi?

jules-ch commented 1 year ago

Oh I see what you mean, to have a warning only for conversion between angular frequency & frequency. Sorry I didn't get that at first. Why not that could be a solution, if the implementation is simple enough that could be something we might consider adding.