-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
gh-137658: Fix dataclass order method behaviors to align with the equality semantics #137497
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
base: main
Are you sure you want to change the base?
Conversation
Don't use a PR number as the issue prefix, things get confused. Please can you create an issue for this PR, and add a NEWS entry? Please can you use a A |
We need to be very careful that we're not changing semantics with this. We've had that problem before with comparisons (although of course now I can't find the issue). I don't think we added tests for this, because we didn't find the problem until after we'd released the code. |
@ericvsmith so we need set of tests that pass prior to this change? Potentially relevant "semantics" you mention may be shown in the tests for this change in #582? |
I'm specifically thinking of #128294. We don't want to do something like that again. I haven't looked at this change closely, I just want to be extra careful. |
@ericvsmith Thanks for that info, I looked into the situation a bit and it seems that since the behavior of 3.13 for I will file an issue for this @dataclasses.dataclass(order=True, slots=True)
class Data:
a: float
class Obj:
__slots__ = "a",
def __init__(self, a: float):
self.a = a
def __eq__(self, other):
if self is other:
return True
if self.__class__ is other.__class__:
return (
self.a == other.a
)
return NotImplemented
def __lt__(self, other):
if self.__class__ is other.__class__:
return self.a < other.a
return NotImplemented
def __le__(self, other):
if self.__class__ is other.__class__:
return self.a <= other.a
return NotImplemented
def __gt__(self, other):
if self.__class__ is other.__class__:
return self.a > other.a
return NotImplemented
def __ge__(self, other):
if self.__class__ is other.__class__:
return self.a >= other.a
return NotImplemented
nan = float("nan")
assert not (Data(nan) < Data(nan))
# assert not (Data(nan) <= Data(nan)), "fails on 3.13 even though {<, ==} are both False"
assert not (Data(nan) > Data(nan))
# assert not (Data(nan) >= Data(nan)), "fails on 3.13 even though {>, ==} are both False"
assert (Data(nan) != Data(nan))
assert not (Data(nan) == Data(nan))
assert not (Obj(nan) < Obj(nan))
assert not (Obj(nan) <= Obj(nan))
assert not (Obj(nan) > Obj(nan))
assert not (Obj(nan) >= Obj(nan))
assert (Obj(nan) != Obj(nan))
assert not (Obj(nan) == Obj(nan)) |
Similar to #104904
This can speed up ordering/comparison operations by ~2x by making the comparison similar to existing
__eq__
performance.Benchmark timings for classes with 5 integer fields: