python / cpython

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

field "mro" behaves strangely in dataclass #89694

Open dc80ef12-04a0-4726-93ca-eaef9ebbf19f opened 3 years ago

dc80ef12-04a0-4726-93ca-eaef9ebbf19f commented 3 years ago
BPO 45531
Nosy @ericvsmith, @merwok, @serhiy-storchaka, @finite-state-machine

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 = ['type-bug', 'library', '3.10'] title = 'field "mro" behaves strangely in dataclass' updated_at = user = 'https://github.com/finite-state-machine' ``` bugs.python.org fields: ```python activity = actor = 'serhiy.storchaka' assignee = 'eric.smith' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'finite-state-machine' dependencies = [] files = [] hgrepos = [] issue_num = 45531 keywords = [] message_count = 9.0 messages = ['404378', '404381', '404414', '404430', '404458', '404819', '404824', '404846', '404857'] nosy_count = 4.0 nosy_names = ['eric.smith', 'eric.araujo', 'serhiy.storchaka', 'finite-state-machine'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue45531' versions = ['Python 3.10'] ```

dc80ef12-04a0-4726-93ca-eaef9ebbf19f commented 3 years ago

The following Python script:

    from dataclasses import dataclass

    @dataclass
    class A:
        mro: object
        x: object

Results in the following unexpected exception:

    Traceback (most recent call last):
      File "/Users/dsuffling/.pyenv/versions/3.10.0/lib/python3.10/runpy.py", line 196, in _run_module_as_main
        return _run_code(code, main_globals, None,
      File "/Users/dsuffling/.pyenv/versions/3.10.0/lib/python3.10/runpy.py", line 86, in _run_code
        exec(code, run_globals)
      File "/Users/dsuffling/names/junk.py", line 6, in <module>
        class A:
      File "/Users/dsuffling/.pyenv/versions/3.10.0/lib/python3.10/dataclasses.py", line 1178, in dataclass
        return wrap(cls)
      File "/Users/dsuffling/.pyenv/versions/3.10.0/lib/python3.10/dataclasses.py", line 1169, in wrap
        return _process_class(cls, init, repr, eq, order, unsafe_hash,
      File "/Users/dsuffling/.pyenv/versions/3.10.0/lib/python3.10/dataclasses.py", line 1019, in _process_class
        _init_fn(all_init_fields,
      File "/Users/dsuffling/.pyenv/versions/3.10.0/lib/python3.10/dataclasses.py", line 540, in _init_fn
        raise TypeError(f'non-default argument {f.name!r} '
    TypeError: non-default argument 'x' follows default argument

The name of the first attribute ('mro') is critical; without it the problem does not occur.

It appears that 'mro' is somehow interacting with the 'mro' attribute of the 'type' object.

This issue has been verified to occur with CPython 3.10.0.

ericvsmith commented 3 years ago

I agree on your analysis. You'll get the same error on any name that type defines (like __class__), but "mro" looks like the only one without dunders.

I'm not sure the best way to fix this. I'll give it some thought.

Another problem is that assigning a default value breaks the .mro() call:

@dataclass
class A:
    mro: object = 3
>>> A.mro()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'int' object is not callable
serhiy-storchaka commented 3 years ago

In Enum it is just implicitly forbidden:

>>> from enum import *
>>> class A(Enum):
...   mro = 1
...   x = 2
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/serhiy/py/cpython/Lib/enum.py", line 430, in __new__
    raise ValueError('Invalid enum member name: {0}'.format(
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: Invalid enum member name: mro
dc80ef12-04a0-4726-93ca-eaef9ebbf19f commented 3 years ago

For what it's worth, I think a sensible exception message solves this problem. While it would be nice to be able to use a field called 'mro', that's an enhancement; the misleading exception message is a bug.

ericvsmith commented 3 years ago

I think the only other thing that could be done is to have a special test for "default is type.mro", and if so, don't assume it's a default value. Which means that you could never actually use:

@dataclass
class A:
   mro: object = type.mro

But it's probably best to just disallow a field named "mro". Which is unfortunate, but such is life. It's a shame mro isn't a builtin, so we could do mro(A) instead of A.mro().

merwok commented 3 years ago

If dataclasses wanted to allow fields named mro, it could replace its call to cls.mro() with type.mro(cls). But I don’t know if there is a strong use case for such a field.

serhiy-storchaka commented 3 years ago

Where does dataclasses call mro()?

ericvsmith commented 3 years ago

The problem is that dataclasses is looking for a default value for a field by looking at getattr(cls, fieldname), which returns a value when fieldname is "mro".

I think the best thing to do, at least for now, is prohibit a field named "mro".

Ultimately I'd like to see cls.mro go away (maybe being replaced by a builtin), but I realize that's not likely to happen.

serhiy-storchaka commented 3 years ago

Would not be more correct to look at cls.__dict__[fieldname]?

BTW, mro() cannot be builtin, because you should be able to override it in some classes.