-
-
Notifications
You must be signed in to change notification settings - Fork 33k
gh-138991: Update dataclass documentation for new eq behavior in Python 3.13 and add tests #139007
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
This comment was marked as resolved.
This comment was marked as resolved.
Doc/library/dataclasses.rst
Outdated
generated. | ||
|
||
.. versionchanged:: 3.13 | ||
The generated ``__eq__`` method now compares each field individually (e.g., ``self.a == other.a and self.b == other.b``), rather than comparing tuples of fields as in previous versions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please wrap lines to 79 characters.
Also, please don't use Latin abbreviations, see the devguide for more information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I’ve made the updates based on your suggestions.
Doc/library/dataclasses.rst
Outdated
comparing tuples of fields as in previous versions. | ||
|
||
This method compares the class by comparing each field in order. Both | ||
instances in the comparison must be of the identical type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this sentence should come before the versionchanged note. It's a description of how the method works, which is more important than how it changed over time.
|
||
In Python 3.12 and earlier, the comparison was performed by creating | ||
tuples of the fields and comparing them (for example, | ||
``(self.a, self.b) == (other.a, other.b)``). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how the formatting would work out, but I think the note about 3.12 should be part of the versionchanged note. Also, might it be worth mentioning why this matters? It would matter for any value V
where V!=V
. Of the built in types/values, I think this only matters for NaN (but I'm willing to be corrected). On the other hand, maybe this would be getting into too much detail for the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For formatting, @StanFromIreland suggested wrapping lines to 79 characters.
I’ll update these changes for now:
-Moving the sentence before the versionchanged note.
For the other changes you suggested, I’d like to wait until they are confirmed before updating.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By "formatting", I meant possibly multiple paragraphs in a versionadded block. I don't know if that's possible (or desirable).
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use more specific unittest methods.
@ericvsmith please review these changes when you get a chance. |
#138991
This PR updates the documentation for the
eq
parameter in thedataclasses
module to reflect the new field-by-field comparison behavior introduced in Python 3.13.A .. versionchanged:: 3.13
notice has been added, and the previous tuple-based comparison for Python 3.12 and earlier is described for clarity.Additionally, new tests have been added to verify the updated
__eq__
implementation, including field-by-field comparison, type checking, and custom field equality logic.Please let me know if my approach or fix needs any improvements . I’m open to feedback and happy to make changes based on suggestions.
Thankyou !
📚 Documentation preview 📚: https://cpython-previews--139007.org.readthedocs.build/