Issue 36077: Inheritance dataclasses fields and default init statement (original) (raw)

Created on 2019-02-22 12:27 by Кирилл Чуркин, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 17322 closed Epic_Wink,2019-11-21 15:36
Messages (11)
msg336297 - (view) Author: Кирилл Чуркин (Кирилл Чуркин) Date: 2019-02-22 12:27
I found a problem when use inherit dataclasses. When I define parent dataclass with field(s) with default (or default_factory) properties, and inherit child dataclass from parent, i define non-default field in it and got `TypeError('non-default argument {f.name!r} follows default argument')` in dataclasses.py(466)._init_fn. It happens because dataclass constructor defines all parent class fields as arguments in __init__ class and then all child class fields. Maybe it need to define all non-default fields in init before all default.
msg336317 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2019-02-22 14:43
Can you please add a simple reproducer to understand the issue in little more detail? I could see some tests along with different cases producing the error message at https://github.com/python/cpython/blob/a40681dd5db8deaf05a635eecb91498dac882aa4/Lib/test/test_dataclasses.py#L55 and a simple script would be helpful in understanding the behavior of the report.
msg336343 - (view) Author: Rémi Lapeyre (remi.lapeyre) * Date: 2019-02-22 19:09
I think this is what is referring Кирилл Чуркин to: Python 3.7.2 (default, Jan 13 2019, 12:50:01) [Clang 10.0.0 (clang-1000.11.45.5)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> from dataclasses import dataclass >>> @dataclass ... class Parent: ... x: int = 1 ... >>> Parent() Parent(x=1) >>> @dataclass ... class Child(Parent): ... y: int ... Traceback (most recent call last): File "", line 2, in File "/usr/local/Cellar/python/3.7.2_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/dataclasses.py", line 991, in dataclass return wrap(_cls) File "/usr/local/Cellar/python/3.7.2_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/dataclasses.py", line 983, in wrap return _process_class(cls, init, repr, eq, order, unsafe_hash, frozen) File "/usr/local/Cellar/python/3.7.2_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/dataclasses.py", line 904, in _process_class else 'self', File "/usr/local/Cellar/python/3.7.2_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/dataclasses.py", line 490, in _init_fn raise TypeError(f'non-default argument {f.name!r} ' TypeError: non-default argument 'y' follows default argument @eric.smith, do you think Child's argument should be merged nicely with Parent's ones in this case? If so, can I propose a PR?
msg336344 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2019-02-22 19:21
I'm not keen on re-ordering parameters. Maybe it could be done if specified with a parameter to @dataclass.
msg336345 - (view) Author: Rémi Lapeyre (remi.lapeyre) * Date: 2019-02-22 19:32
I see your point. On the other hand, a new parameter would also increase the complexity for the user. Maybe it should not be seen as re-ordering but just a "zipping" them correctly: @dataclass class Parent: i: int j: int = 0 @dataclass class Child(Parent): k: int l: int = 1 The "naive" to define Child's __index__ is: __index__(self, i: int, j: int = 0, k: int, l: int = 1): but wouldn't this make sense (given that it is previsible and deterministic)? __index__(self, i: int, k: int, j: int = 0, l: int = 1):
msg348920 - (view) Author: Daniel Lepage (dplepage) Date: 2019-08-02 21:05
A simpler way to merge them would be to make all arguments after a default argument keyword-only, e.g. __index__(self, i, j=0, *, k, l=0) It does mean you'd have to explicitly write e.g. Child(1, k=4), but that's a lot more readable than seeing Child(1, 4) and wondering which field gets the 4.
msg357178 - (view) Author: Laurie Opperman (Epic_Wink) * Date: 2019-11-21 15:39
I've added a PR implementing Daniel L's suggestion
msg357855 - (view) Author: Martijn Pieters (mjpieters) * Date: 2019-12-05 15:44
I've supported people hitting this issue before (see https://stackoverflow.com/a/53085935/100297, where I used a series of mixin classes to make use of the changing MRO when the mixins share base classes, to enforce a field order from inherited classes. I'd be very much in favour of dataclasses using the attrs approach to field order: any field named in a base class *moves to the end*, so you can 'insert' your own fields by repeating parent fields that need to come later: @attr.s(auto_attribs=True) class Parent: foo: str bar: int baz: bool = False @attr.s(auto_attribs=True) class Child(Parent): spam: str baz: bool = False The above gives you a `Child(foo: str, bar: int, spam: str, baz: bool = False)` object, note that `baz` moved to the end of the arguments. `dataclasses` currently doesn't do this, so it'd be a breaking change.
msg368315 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2020-05-06 23:35
It would be good if there were some way of unifying existing usage with positional-only and keyword-only parameters, and also supporting inheritance for dataclasses that use these features at various points in the hierarchy. I don't have any big ideas about this. And of course it all needs to be backward compatible.
msg368320 - (view) Author: Laurie Opperman (Epic_Wink) * Date: 2020-05-07 01:48
Daniel's suggestion (and my PR) introduce a mechanism that is as far as I know almost completely bakwards-compatible. The only issue is if people were wanting (and acting on) a TypeError to be raised on dataclass construction (which I would say is rare to non-existant), and is the issue raised by the original post.
msg392371 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2021-04-30 00:46
I'm going to close this in favor of the kw_only work done in issue 43532. I realize they're not quite the same thing, but we'll see how it works out in Python 3.10, and possibly make adjustments when we have some real world experience.
History
Date User Action Args
2022-04-11 14:59:11 admin set github: 80258
2021-04-30 00:46:33 eric.smith set status: open -> closedresolution: wont fixmessages: + stage: patch review -> resolved
2020-05-07 01:48:03 Epic_Wink set messages: +
2020-05-06 23:35:13 eric.smith set messages: +
2019-12-05 15:44:44 mjpieters set nosy: + mjpietersmessages: +
2019-11-21 15:39:00 Epic_Wink set nosy: + Epic_Winkmessages: + versions: + Python 3.6, - Python 3.7, Python 3.8
2019-11-21 15:36:58 Epic_Wink set keywords: + patchstage: patch reviewpull_requests: + <pull%5Frequest16808>
2019-08-13 13:09:51 kgustyr set nosy: + kgustyr
2019-08-02 21:05:02 dplepage set nosy: + dplepagemessages: +
2019-02-22 19:32:53 remi.lapeyre set messages: +
2019-02-22 19:21:22 eric.smith set messages: +
2019-02-22 19:09:58 remi.lapeyre set nosy: + remi.lapeyremessages: +
2019-02-22 14:44:25 eric.smith set assignee: eric.smith
2019-02-22 14:43:45 xtreak set nosy: + xtreakmessages: + components: + Library (Lib)
2019-02-22 12:28:34 SilentGhost set nosy: + eric.smithversions: + Python 3.8
2019-02-22 12:27:13 Кирилл Чуркин create