bpo-43558: Add base class initialization to dataclass doc#25967
Conversation
This adds an sentence explaining that `super().__init__` ought to be called in the `__post_init__` function of dataclasses, and it adds a code example, since I felt it was clearer than a textual describtion.
| def __post_init__(self): | ||
| self.c = self.a + self.b | ||
|
|
||
| If the dataclass inherits from another class, which defines an :meth:`__init__` |
There was a problem hiding this comment.
I'd like to start with something like: "The __init__ function generated by @dataclass does not call base class __init__ methods. If base class __init__ methods need to be called, it's common to do so in a __post_init__ method."
There was a problem hiding this comment.
Thanks for the feedback! I have adapted this text slightly in my last commit to reflect that the __init__ ought to be called if the base class has a non-default __init__ method, i.e. the one defined on object (this is also mentioned here in the docs). I was wondering if it should be mentioned that, when dealing with dataclasses, this is likely the case (I think that the generation of __init__ by @dataclass might obscure the presence of a non-default __init__).
|
|
||
| def __post_init__(self): | ||
| super().__init__(self.side, self.side) | ||
|
|
|
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 |
d24735b to
da2f0de
Compare
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @ericvsmith: please review the changes made to this pull request. |
There was a problem hiding this comment.
I don't know that anyone would know what "non-default __init__ method" means. I'd just leave it as "an __init__ method that needs to be called", or similar. Or maybe add words that say that in general, dataclass-generated __init__ methods to not need to be explicitly called, because the derived class __init__ takes care of initializing all fields.
da2f0de to
62f111c
Compare
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @ericvsmith: please review the changes made to this pull request. |
There was a problem hiding this comment.
This looks good. How about instead of "the derived class", use "the derived dataclass"? I'm trying to draw the distinction that if a non-dataclass is derived from a dataclass, then the base class's __init__ should in fact be called.
There was a problem hiding this comment.
If I understand PEP 557 correctly, the __init__ method needs to be called if a dataclass inherits from a non-dataclass or if a non-dataclass inherits from a dataclass. I have tried to reflect this in the new wording.
There was a problem hiding this comment.
If I understand PEP 557 correctly, the
__init__method needs to be called if a dataclass inherits from a non-dataclass or if a non-dataclass inherits from a dataclass. I have tried to reflect this in the new wording.
If a dataclass inherits from a non-dataclass, then it's probably that the base non-dataclass's __init__ does need to be called. That's what __post_init__ is for, among other uses.
If a non-dataclass inherits from a dataclass, then the non-dataclass most likely does need to call the dataclass's __init__.
It's only the case where a dataclass inherits from a dataclass that the base class's __init__ most likely doesn't need to be called. I say "most likely" because you could construct examples where it does need to be called, if the base dataclass's __post_init__ does some processing. But it's probably too complex to mention that here, I'm just mentioning it here to help you understand the issue. Apologies if I'm not being helpful.
There was a problem hiding this comment.
This is very helpful! Thank you!
9cac499 to
004f915
Compare
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @ericvsmith: please review the changes made to this pull request. |
ericvsmith
left a comment
There was a problem hiding this comment.
Thank you for your patience. I think we're close.
You might want to add your name to the Misc/ACKS file.
There was a problem hiding this comment.
"not need to" isn't correct. Apologies if that was my error. Maybe simpler language is needed, like "... methods don't need to be called, since ...`.
There was a problem hiding this comment.
Sorry, English is not my native tongue
There was a problem hiding this comment.
If I understand PEP 557 correctly, the
__init__method needs to be called if a dataclass inherits from a non-dataclass or if a non-dataclass inherits from a dataclass. I have tried to reflect this in the new wording.
If a dataclass inherits from a non-dataclass, then it's probably that the base non-dataclass's __init__ does need to be called. That's what __post_init__ is for, among other uses.
If a non-dataclass inherits from a dataclass, then the non-dataclass most likely does need to call the dataclass's __init__.
It's only the case where a dataclass inherits from a dataclass that the base class's __init__ most likely doesn't need to be called. I say "most likely" because you could construct examples where it does need to be called, if the base dataclass's __post_init__ does some processing. But it's probably too complex to mention that here, I'm just mentioning it here to help you understand the issue. Apologies if I'm not being helpful.
|
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 |
004f915 to
ddea3b4
Compare
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @ericvsmith: please review the changes made to this pull request. |
|
Thanks @dhoekstra2000 for the PR, and @ericvsmith for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.9. |
|
GH-26018 is a backport of this pull request to the 3.10 branch. |
|
GH-26019 is a backport of this pull request to the 3.9 branch. |
pythonGH-25967) (cherry picked from commit 2a03172) Co-authored-by: dhoekstra2000 <douwe.hoekstra2512@gmail.com>
This adds an sentence explaining that
super().__init__oughtto be called in the
__post_init__function of dataclasses, andit adds a code example, since I felt it was clearer than a
textual describtion.
https://bugs.python.org/issue43558