Skip to content

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

gesslerpd
Copy link
Contributor

@gesslerpd gesslerpd commented Aug 7, 2025

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:

equality (ms) order (ms)
standard class w/ optimization 46.19 47.27
existing dataclasses 45.96 114.22

@gesslerpd gesslerpd requested a review from ericvsmith as a code owner August 7, 2025 00:31
@gesslerpd gesslerpd changed the title gh-*: Performance optimization for dataclass order methods gh-137497: Performance optimization for dataclass order methods Aug 7, 2025
@AA-Turner AA-Turner changed the title gh-137497: Performance optimization for dataclass order methods Performance optimization for dataclass order methods Aug 7, 2025
@AA-Turner
Copy link
Member

AA-Turner commented Aug 7, 2025

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 pyperformance benchmark, and update the PR with the benchmark code and results? Microbenchmark results should show a reasonably large performance improvement, which this PR hopefully will based on the preliminary numbers you've shown.

A

@ericvsmith
Copy link
Member

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.

@gesslerpd
Copy link
Contributor Author

@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?

@ericvsmith
Copy link
Member

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.

@gesslerpd
Copy link
Contributor Author

gesslerpd commented Aug 12, 2025

@ericvsmith Thanks for that info, I looked into the situation a bit and it seems that since the behavior of 3.13 for __eq__ is accepted as intended now that this may actually fix things to be consistent with that.

I will file an issue for this order=True discrepancy on 3.13 and link this PR.

@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))

@gesslerpd gesslerpd changed the title Performance optimization for dataclass order methods gh-137658: Fix dataclass order method behaviors to align with the equality semantics Aug 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants