pint
pint copied to clipboard
Offset Unit Multiplication
Just a thought here.
We had a lot of questions regarding offset units particularly when using multiplication with unit.
Like :
15 * ureg.deg_C
Raising an error instead of Quantity(15, "deg_C")
In the same direction, I do remember @jthielen & @dopplershift having performance issues in metpy when using this form to initialize Quantity. And had to fallback using the constructor.
Wouldn't be possible to have this form be equivalent to initializing with the Quantity
constructor:
currently : 15 * ureg.deg_C
<=> Quantity(1, "deg_C") * 15
to : 15 * ureg.deg_C
<=> Quantity(15, "deg_C")
We will get rid of the performance issues & any confusion when using this simple form. 2 birds, 1 stone.
Regarding security, we would still have those offset checks when making any multiplication or division with:
- Quantity & Units
- Quantity & Quantity
In the documentation, this sentence still stands with this change & will only affect the simple form of value * unit
.
Any further operation would fall under the rule of Quantity multiplication.
However, multiplication, division and exponentiation of quantities with offset units is problematic just like addition. Pint (since version 0.6) will by default raise an error when a quantity with offset unit is used in these operations. Due to this quantities with offset units cannot be created like other quantities by multiplication of magnitude and unit but have to be explicitly created
Any thoughts @hgrecco, @keewis or use cases I haven't thought about ? When making this change, I see no test failing.
The current implementation:
def __mul__(self, other):
if self._check(other):
if isinstance(other, self.__class__):
return self.__class__(self._units * other._units)
else:
qself = self._REGISTRY.Quantity(1, self._units)
return qself * other
if isinstance(other, Number) and other == 1:
return self._REGISTRY.Quantity(other, self._units)
return self._REGISTRY.Quantity(1, self._units) * other
And as you can see 1 * ureg.degC
will return when magnitude is 1. 😄
I would strongly disfavour making multiplication a synonym with construction for offset units. There is no physical meaning to multiplication with what we call here "offset units" and are really quantity coordinates. The multiplication sign intuitively represents an associative operation, but with the proposed changes you could do (2 * 4) * u.deg_C
but not 2 * (4 * u.deg_C)
. If you have generic code that is constructing a quantity from unit and value, you can always do u.Quantity(value,unit)
, that is explicit and unambiguous.
I wasn't aware that there is a special-case for "1" though. What is the reason for that special case? That feels like a hacky work-around for some other fundmental problem/misconception. I can see all sorts of potential subtle bugs and surprises with that special case. I think that one should be removed.
Btw, I have an open PR to enable parsing of offset units from strings, and that one only allows the form without the multiplication.
What about removing offset-unit multiplication completely and accept only the explicit creation via u.Quantity(value,unit)
? This would solve the problem at the basis. Of course such a change should not happen from one version to the next.
I am all in favour of removing/disallowing multiplication of offset units under any circumstance. However, I'm not sure I understand what exactly you are suggesting here. It is currently already disallowed completely, except for a registry flag that re-enables such multiplications. And then there is the special case above for numeric values equal to 1. If you mean to remove that special case, that's what I'm saying.