classification
Title: Inheritance dataclasses fields and default init statement
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: eric.smith Nosy List: Epic_Wink, dplepage, eric.smith, kgustyr, mjpieters, remi.lapeyre, xtreak, Кирилл Чуркин
Priority: normal Keywords: patch

Created on 2019-02-22 12:27 by Кирилл Чуркин, last changed 2021-04-30 00:46 by eric.smith. 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 "<stdin>", line 2, in <module>
  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
2021-04-30 00:46:33eric.smithsetstatus: open -> closed
resolution: wont fix
messages: + msg392371

stage: patch review -> resolved
2020-05-07 01:48:03Epic_Winksetmessages: + msg368320
2020-05-06 23:35:13eric.smithsetmessages: + msg368315
2019-12-05 15:44:44mjpieterssetnosy: + mjpieters
messages: + msg357855
2019-11-21 15:39:00Epic_Winksetnosy: + Epic_Wink

messages: + msg357178
versions: + Python 3.6, - Python 3.7, Python 3.8
2019-11-21 15:36:58Epic_Winksetkeywords: + patch
stage: patch review
pull_requests: + pull_request16808
2019-08-13 13:09:51kgustyrsetnosy: + kgustyr
2019-08-02 21:05:02dplepagesetnosy: + dplepage
messages: + msg348920
2019-02-22 19:32:53remi.lapeyresetmessages: + msg336345
2019-02-22 19:21:22eric.smithsetmessages: + msg336344
2019-02-22 19:09:58remi.lapeyresetnosy: + remi.lapeyre
messages: + msg336343
2019-02-22 14:44:25eric.smithsetassignee: eric.smith
2019-02-22 14:43:45xtreaksetnosy: + xtreak
messages: + msg336317
components: + Library (Lib)
2019-02-22 12:28:34SilentGhostsetnosy: + eric.smith

versions: + Python 3.8
2019-02-22 12:27:13Кирилл Чуркинcreate