-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Fix dataclass inherited field ordering #5877
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
Fix dataclass inherited field ordering #5877
Conversation
27f1fee
to
ac93608
Compare
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 for PR! This looks good, I have few comments below. Also I have two questions:
- Is this specific to dataclasses, or
attrs
plugin should be also updated to give correct order? - Do we have an issue about this? If yes, I would add
Fixes #XXXX
to the top of PR description.
# reverse MRO, not simply MRO. | ||
# See https://docs.python.org/3/library/dataclasses.html#inheritance for | ||
# details. | ||
(attr,) = [a for a in all_attrs if a.name == name] |
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.
Will this not crash if there are two attrs with the same name? Do we have a test for this?
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 believe it can happen within a single class and with multiple classes the if name not in known_attrs
block either adds an attribute that wasn't there or, in case of an attribute being overridden, it reorders attributes without adding duplicates.
The way this assignment is written is to express the fact that we strictly expect single attribute with the name
name here, I can't think of a situation in which this condition is not met, any possible name clashes are covered by the tests already.
ac4bb0d
to
ac97971
Compare
I have no idea about attrs and the attrs plugin, I've never used it. Turns out there was an issue filled about more or less the same thing - I updated the description and the commit message. |
This is how the dataclasses module behaves on Python 3.7.0: % cat test.py from dataclasses import dataclass @DataClass class Mammal: age: int @DataClass class Person(Mammal): name: str age: int @DataClass class SpecialPerson(Person): special_factor: float @DataClass class ExtraSpecialPerson(SpecialPerson): age: int special_factor: float name: str print(Person(32, 'John')) print(SpecialPerson(21, 'John', 0.5)) print(ExtraSpecialPerson(21, 'John', 0.5)) % python test.py Person(age=32, name='John') SpecialPerson(age=21, name='John', special_factor=0.5) ExtraSpecialPerson(age=21, name='John', special_factor=0.5) Fixes python#5681.
ac97971
to
b961e4a
Compare
Hey @jstasiak , @ilevkivskyi , For some reason I keep getting the same issue, I'm using my 0.670.
|
Can you provide a complete piece of code that works at runtime yet triggers this? I think there are some pieces missing here (no |
@jstasiak it looks like it was an internal issue. Solved, Tx. |
Fixes #5681.
This is how the dataclasses module behaves on Python 3.7.0: