classification
Title: dataclass MRO entry resolution for type variable metaclasses: TypeError
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Ricyteach, eric.smith, levkivskyi
Priority: normal Keywords: patch

Created on 2018-03-30 18:03 by Ricyteach, last changed 2018-04-06 21:31 by levkivskyi. This issue is now closed.

Files
File name Uploaded Description Edit
dataclass_metaclass_issue.py Ricyteach, 2018-03-30 18:03 demo of problem
Pull Requests
URL Status Linked Edit
PR 6319 merged levkivskyi, 2018-03-31 10:46
PR 6320 merged miss-islington, 2018-03-31 12:43
Messages (11)
msg314703 - (view) Author: Rick Teachey (Ricyteach) * Date: 2018-03-30 18:03
I'm getting the following error at when attempting to create a new `dataclass` with any base class that is supplied a type variable:

    TypeError: type() doesn't support MRO entry resolution; use types.new_class()

In principle, it seems like this shouldn't cause any problems, since this works as expected:

    @dataclass
    class MyClass(Generic[MyTypeVar]):
        pass

The problem seems to be the call to `type` in `make_dataclass` for instantiating the class object, since `type` doesn't support type variables. If I replace the `dataclass` line that produces the error with the following code block, it seems to work:

    try:
        cls = type(cls_name, bases, namespace)
    except TypeError:
        cls = types.new_class(cls_name, bases, namespace)

I haven't tested, but it might be possible to just remove the call to `type` altogether.

I've attached a file demonstrating the issue.
msg314704 - (view) Author: Rick Teachey (Ricyteach) * Date: 2018-03-30 18:21
Sorry: to be clear, the error only occurs when attempting to create the class using `make_dataclass`.
msg314708 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-03-30 19:52
You can also cause this same error without dataclasses:

from typing import TypeVar, Generic
from types import new_class
MyTypeVar = TypeVar("MyTypeVar")
MyParent = new_class("MyParent", (Generic[MyTypeVar],), {})
c = type('MyChild', (MyParent[int],), {})  # error in 3.8

I assume you get the same error in 3.7, but I can't compile it just now.

The above code works in 3.6. And the code in dataclass_metaclass_issue.py works in 3.6, using the backported dataclasses.py from PyPI.

So it seems this is a typing problem, not a dataclasses problem.

+Ivan, in case he has some insights.
msg314709 - (view) Author: Rick Teachey (Ricyteach) * Date: 2018-03-30 20:02
Same error on 3.7.

Probably getting beyond my knowledge here but from the error message, it seems like the answer is simply that:

type('MyChild', (MyParent[int],), {})

...is just the wrong way to make a new `type` when utilizing type variables.
msg314710 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2018-03-30 20:07
This is not a bug but an explicit design decision. Generic classes are _static_ typing concept and therefore are not supposed to work freely with _dynamic_ class creation. During discussion of PEP 560 it was decided that there should be at least one way to dynamically create generic classes, `types.new_class` was chosen for this, see https://www.python.org/dev/peps/pep-0560/#dynamic-class-creation-and-types-resolve-bases

Also the exception message is quite clear about this. Unfortunately, PEPs 560 and 557 were discussed in parallel so not every possible interactions where thought out. But is it critical for dataclasses to call `type`? I believe there should be no other differences with `types.new_class`. I would say the latter is even better than `type` because it correctly treats `__prepare__` on the metaclass IIRC. So I would propose to switch from `type()` to `types.new_class()` for dynamic creation of dataclasses.
msg314721 - (view) Author: Rick Teachey (Ricyteach) * Date: 2018-03-30 21:12
Eric: see also Ivan's comment on Issue 33190, which will factor into the solution to this I think. It seems you can't just pass the `namespace` to the `kwds` argument (as I have done in my solution above).
msg314724 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-03-30 22:41
If we're going to call new_class in make_dataclass, then we should change the signature of make_dataclass to have the new_class parameters.
msg314726 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2018-03-30 22:51
> If we're going to call new_class in make_dataclass, then we should change the signature of make_dataclass to have the new_class parameters.

Why? I think we should care about what API/signature is reasonable/typical for dataclasses use cases, while new_class is really just an implementation detail. IMO passing keyword arguments to metaclass will be much more rare for dataclasses than passing a ready namespace.
msg314728 - (view) Author: Rick Teachey (Ricyteach) * Date: 2018-03-31 01:01
> passing keyword arguments to metaclass will be much more rare for dataclasses than passing a ready namespace

The impetus of my running into these issues was assuming that things like `Generic[MyTypeVar]` would "just work" with `make_dataclass`, which seemed like a valid assumption since the class creation approach made heavy use of by `dataclasses` implies this:

@dataclass
class MyDclass(Generic[MyTypeVar]):
    var: MyTypeVar

The fact that I cannot do this, then, without error is surprising:

MyDclass = make_dataclass("MyDclass", (("var", MyTypeVar),), bases=(Generic[MyTypeVar],))

I'm not stating it HAS to be fixed. Maybe it doesn't have to. But to me, the above seems like the reason to do it if it's going to be done.
msg314729 - (view) Author: Rick Teachey (Ricyteach) * Date: 2018-03-31 01:05
I'll also say: one of the biggest reasons I was excited to read the `dataclasses` PEP was because I thought "AWESOME! Now I can delete all of the stupid boilerplate __init__ and __repr__ methods for those `typing.Sequences`, `typing.Mapping`, etc etc subclasses!"
msg314738 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-03-31 12:21
Yeah, no need to change the API. I was over-generalizing.
History
Date User Action Args
2018-04-06 21:31:36levkivskyisetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-03-31 12:43:02miss-islingtonsetpull_requests: + pull_request6036
2018-03-31 12:21:34eric.smithsetmessages: + msg314738
2018-03-31 10:46:25levkivskyisetkeywords: + patch
stage: patch review
pull_requests: + pull_request6035
2018-03-31 01:05:02Ricyteachsetmessages: + msg314729
2018-03-31 01:01:31Ricyteachsetmessages: + msg314728
2018-03-30 22:51:39levkivskyisetmessages: + msg314726
2018-03-30 22:41:22eric.smithsetmessages: + msg314724
2018-03-30 21:12:12Ricyteachsetmessages: + msg314721
2018-03-30 20:07:43levkivskyisetmessages: + msg314710
2018-03-30 20:02:12Ricyteachsetmessages: + msg314709
2018-03-30 19:52:17eric.smithsetnosy: + levkivskyi
messages: + msg314708
2018-03-30 18:21:21Ricyteachsetmessages: + msg314704
2018-03-30 18:03:43Ricyteachcreate