Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Confusing error about __attrs_attrs__ when using multiple inheritance #12065

Closed
JukkaL opened this issue Jan 25, 2022 · 5 comments · Fixed by #12411
Closed

Confusing error about __attrs_attrs__ when using multiple inheritance #12065

JukkaL opened this issue Jan 25, 2022 · 5 comments · Fixed by #12411

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 25, 2022

This code now generates an error about class AB:

import attr

@attr.s
class A:
    x = attr.ib(type=int)

@attr.s
class B:
    y = attr.ib(type=int)

# Definition of "__attrs_attrs__" in base class "A" is incompatible with definition in base class "B"
class AB(A, B):
    pass

The code is arguably questionable, and the error goes away if we decorate AB with @attr.s, but that's not clear from the error message. It would be better to add a note, such as something like this:

... :note: Should class "AB" use the @attr.s decorator?

This started happening after #11794, and it might be considered a minor regression because of the confusing message.

(Behind the scenes, a __attrs_attrs__ attribute gets generated with a class-specific named tuple as the type. These named tuples aren't compatible with each other.)

cc @Tinche

@Tinche
Copy link
Contributor

Tinche commented Jan 25, 2022

Interesting, I can take a look at this.

@Tinche
Copy link
Contributor

Tinche commented Jan 25, 2022

In runtime,

>>> AB.__attrs_attrs__
(Attribute(name='x', default=NOTHING, validator=None, repr=True, eq=True, eq_key=None, order=True, order_key=None, hash=None, init=True, metadata=mappingproxy({}), type=<class 'int'>, converter=None, kw_only=False, inherited=False, on_setattr=None),)

So I'm guessing it just follows the MRO and gets the class attribute from A.

Here's the equivalent situation without attrs:

class C:
    _test_: str = "a"


class D:
    _test_: int = 1


class CD(C, D):
    pass

This is a Mypy error (error: Definition of "_test_" in base class "C" is incompatible with definition in base class "D").

So if we want to keep this behavior and still raise an error, the note approach is the way to go?

@JukkaL
Copy link
Collaborator Author

JukkaL commented Jan 25, 2022

Yeah, the note approach seems fine. If you don't use @attr.s in the derived class, the inherited __init__ doesn't initialize all the attributes, the value of __attrs_attrs__ won't include all attributes, etc.

@JelleZijlstra
Copy link
Member

This feels similar to #12008. (Does attrs autocreate __match_args__?)

@Tinche
Copy link
Contributor

Tinche commented Jan 25, 2022

This feels similar to #12008. (Does attrs autocreate __match_args__?)

It does.

@JukkaL JukkaL self-assigned this Mar 21, 2022
JukkaL added a commit that referenced this issue Mar 21, 2022
…sses

Multiple inheritance from dataclasses and attrs classes works at runtime,
so don't complain about `__match_args__` or `__attrs_attrs__` which tend
to have incompatible types in subclasses.

Fixes #12349. Fixes #12008. Fixes #12065.
JukkaL added a commit that referenced this issue Mar 22, 2022
Multiple inheritance from dataclasses and attrs classes works at runtime,
so don't complain about `__match_args__` or `__attrs_attrs__` which tend
to have incompatible types in subclasses.

Fixes #12349. Fixes #12008. Fixes #12065.
JukkaL added a commit that referenced this issue Mar 23, 2022
Multiple inheritance from dataclasses and attrs classes works at runtime,
so don't complain about `__match_args__` or `__attrs_attrs__` which tend
to have incompatible types in subclasses.

Fixes #12349. Fixes #12008. Fixes #12065.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants