python / cpython

The Python programming language
https://www.python.org
Other
62.67k stars 30.06k forks source link

dataclass __post_init__ recursion #91094

Open 9326c04b-4d37-4a35-bdd5-d40f5f681a02 opened 2 years ago

9326c04b-4d37-4a35-bdd5-d40f5f681a02 commented 2 years ago
BPO 46938
Nosy @ericvsmith, @bharel

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields: ```python assignee = 'https://github.com/ericvsmith' closed_at = None created_at = labels = ['3.11', 'type-bug', 'library', 'docs'] title = 'dataclass __post_init__ recursion' updated_at = user = 'https://github.com/bharel' ``` bugs.python.org fields: ```python activity = actor = 'eric.smith' assignee = 'eric.smith' closed = False closed_date = None closer = None components = ['Documentation', 'Library (Lib)'] creation = creator = 'bar.harel' dependencies = [] files = [] hgrepos = [] issue_num = 46938 keywords = [] message_count = 5.0 messages = ['414613', '414619', '414626', '414627', '414628'] nosy_count = 3.0 nosy_names = ['eric.smith', 'docs@python', 'bar.harel'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue46938' versions = ['Python 3.11'] ```

9326c04b-4d37-4a35-bdd5-d40f5f681a02 commented 2 years ago

Not sure if a continuance of https://bugs.python.org/issue44365 or not, but the suggestion to call super().__init() in __postinit will cause infinite recursion if the superclass also contains __postinit__:

>>> @d
... class A:
...  test: int
...  def __post_init__(self):
...    pass

>>> @d
... class B(A):
...  test2: int
...  def __post_init__(self):
...    super().__init__(test=1)

>> B(test2=1, test=3) \<-- infinite recursion.

This is caused by line 564 (https://github.com/python/cpython/blob/4716f70c8543d12d18c64677af650d479b99edac/Lib/dataclasses.py#L564) checking for post init on current class, and calling it on self (child class).

Not sure if the bug is in the documentation/suggestion, or if it's a bug in the implementation, in which case we need to call the current class's __post_init__.

ericvsmith commented 2 years ago

I think this is a bug in the code. I'll have a PR ready shortly.

But since it's a non-trivial change, I'm going to target it for 3.11 only.

9326c04b-4d37-4a35-bdd5-d40f5f681a02 commented 2 years ago

@Eric, I can easily fix it if you wish :-)

Just wondered if it's intended or not, as it looked like a bug but the documentation was somewhat endorsing it and I got confused.

Either case, a simple fix.

9326c04b-4d37-4a35-bdd5-d40f5f681a02 commented 2 years ago

Actually I'm not sure if the fix is so simple. What happens if B does not inherit from dataclass, but still inherits from A? Do we want to use the new __post_init? I would assume we do, meaning we don't necessarily want to attach the __post_init to the dataclass, but to subclasses.

If so, the suggestion to call __init() from __post_init() might be the problematic part, and is what conceptually breaks the inheritance linearization.

ericvsmith commented 2 years ago

Yeah, I've come to the conclusion that it's not so simple, either. I'm also thinking that advising to call the base __init__ is a mistake.