Fix incorrect self-comparison for InfinityType and NegativeInfinityType#1093
Fix incorrect self-comparison for InfinityType and NegativeInfinityType#1093
Conversation
|
These are internal data structures. Provide a real public facing issue this creates. Also you must include tests that show the behavioral difference. |
|
Fair point, through normal version comparison these are wrapped in tuples, and tuple That said, the operators are still mathematically wrong on their own: I've added self-comparison tests covering all four operators for both types. Happy to close this if you feel it's not worth the change though. |
|
But calls to isinstance are significantly more expeensive, if we never hit this code path so the solution is only for hypotheticals I'd prefer we raise NotImplemented. |
|
Good point about the cost. I can swap the |
That would be better, I won't be able to review this any time soon though as I'd need to understand where and how this is being used. I'm not against it though, with these fixes. |
|
It would be nice to have these be a little more correct, just in case they are used someday elsewhere, as long as it's not slower, so I'd be for it. |
|
Updated to use |
InfinityType.__le__ unconditionally returns False and __gt__ unconditionally returns True, which gives wrong results for self-comparison: Infinity <= Infinity returns False (should be True) and Infinity > Infinity returns True (should be False). The same issue exists for NegativeInfinityType where __lt__ always returns True and __ge__ always returns False, making NegativeInfinity < NegativeInfinity return True and NegativeInfinity >= NegativeInfinity return False. Fix by checking isinstance for these four operators, consistent with how __eq__ is already implemented.
Since Infinity and NegativeInfinity are singletons, 'other is self' is both cheaper and more correct than isinstance checks.
96fcad5 to
b090b04
Compare
Summary
InfinityTypeandNegativeInfinityTypegive incorrect results when compared to themselves.Problem
Four comparison operators return wrong values for self-comparison:
Infinity <= InfinityFalseTrueInfinity > InfinityTrueFalseNegativeInfinity < NegativeInfinityTrueFalseNegativeInfinity >= NegativeInfinityFalseTrueThese are singletons, so while self-comparison is an edge case, the operators should still be mathematically consistent. Currently
__le__returnsFalseunconditionally forInfinityType, and__gt__returnsTrueunconditionally — neither accounts for self-comparison. The same pattern affectsNegativeInfinityTypefor__lt__and__ge__.Note that
__eq__already handles this correctly usingisinstance(other, self.__class__).Fix
Apply the same
isinstancepattern to the four affected operators:InfinityType.__le__: returnisinstance(other, self.__class__)InfinityType.__gt__: returnnot isinstance(other, self.__class__)NegativeInfinityType.__lt__: returnnot isinstance(other, self.__class__)NegativeInfinityType.__ge__: returnisinstance(other, self.__class__)All other comparisons (cross-type, with regular values) remain unchanged.