Repository navigation
Quantity comparison to dimensionless zero has an amusing edge case #15103
Description
Description
Comparing a quantity with units to dimensionless zero always works, presumably because for a given comparison it is reasonable to assume that dimensionless zero is the same as zero times a unit. However, this results in an amusing edge case when working with two units where the zero point does not coincide. For example, you can't normally compare degrees Celsius and Kelvin without the temperature() equivalency:
>>> -1*u.Celsius < 1*u.Kelvin
...
UnitConversionError: 'K' (temperature) and 'deg_C' (temperature) are not convertible
...
>>> -1*u.Celsius < (1*u.Kelvin).to(u.Celsius, equivalencies=u.temperature())
FalseBut, if you throw dimensionless zero into the mix, you seemingly can compare them without the equivalency (through transitivity) and then obtain the wrong answer.
>>> -1*u.Celsius < 0
True
>>> 1*u.Kelvin > 0
True
>>> -1*u.Celsius < 0 < 1*u.Kelvin
TrueExpected behavior
Something not amusing, but this example is rather contrived
How to Reproduce
See description
Versions
Windows-10-10.0.22621-SP0
Python 3.10.10 | packaged by conda-forge | (main, Mar 24 2023, 20:00:38) [MSC v.1934 64 bit (AMD64)]
astropy 5.2.2
Numpy 1.24.3
pyerfa 2.0.0.3
Scipy 1.10.1
Matplotlib 3.7.1
Activity
roytsmart commentedon Jul 31, 2023
This seems particularly hard to solve since the chained comparison operators are just syntactic sugar for
(a < b) and (b < c), which would also have the same amusing behavior.Seems like the only way out of this is to disable comparisons with dimensionless zero, but that would probably break a lot of downstream code.
Maybe the rich comparison chaining in PEP 535 would help?
ayshih commentedon Aug 4, 2023
Heh, the example in my original report was inspired by a live situation, but it's even more amusing since of course the units compared don't have to be related at all:
pllim commentedon Aug 4, 2023
Maybe don't use
==twice in the same statement? This is Python specific thing right? Not all languages even support such comparison.mhvk commentedon Aug 6, 2023
I think the comparison with degrees C should just fail, i.e., any unit with an offset should not be allowed to compare with 0 without units.
In principle, this is not super-hard -
deg_Canddeg_Fshould become instances of a new function unit class (sayOffsetUnit), which could disable this. But it needs time to implement...ayshih commentedon Aug 6, 2023
Well, as @byrdie noted, it's just syntactic sugar, until there is rich comparison chaining. As a stilted example, one could have a convenience function that implicitly leans on transitivity:
And then argument order matters:
Sure, but that doesn't address the equality comparisons between unrelated units without an offset:
pllim commentedon Aug 6, 2023
Do you think acknowledging this limitation in Known Issues would help?
mhvk commentedon Aug 6, 2023
Surely doesn't hurt. In the end, the reason for allowing
0to have any dimension was simply that many things just worked a little easier (including some numpy functions such asnp.sinc-- though these days one can just override).pllim commentedon Aug 10, 2023
Okay, I proposed text for known issue at #15154 . Do we keep this open even after that, or close this as "won't fix" ?
roytsmart commentedon Aug 11, 2023
I personally think it would be best to deprecate comparison with dimensionless zero, but I'd be interested to hear what others think about it. I'm sure it would cause lots of problems downstream.
mhvk commentedon Aug 11, 2023
See #1254 for why this was added. Personally, I'm not sure I would decide the same way now, but it would definitely seem to be a case that one breaks a lot of code just for being slightly more "correct" -- which I don't think is worth it.
pllim commentedon Aug 11, 2023
But I like that feature!
mhvk commentedon Aug 11, 2023
I think it is super-unlikely we can do much more than the warning @pllim added, so labelled this
close?. I certainly have code that relies on being able to compare with 0 myself...github-actions commentedon Aug 12, 2023
Hi humans 👋 - this issue was labeled as Close? approximately 13 hours ago. If you think this issue should not be closed, a maintainer should remove the Close? label - otherwise, I will close this issue in 7 days.
If you believe I commented on this issue incorrectly, please report this here
github-actions commentedon Aug 20, 2023
I'm going to close this issue as per my previous message, but if you feel that this issue should stay open, then feel free to re-open and remove the Close? label.
If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here