[proxy] web.archive.org← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ mypy Public
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

Fix bugs where overriding init in a dataclass subclass crashed mypy #8159

Merged
merged 2 commits into from Dec 19, 2019

Conversation

Copy link
Contributor

beckjake commented Dec 17, 2019

Fixes #8015
Fixes #8022

This fixes two bugs related to dataclass InitVars and overriding __init__
by changing mypy to ignore InitVars in parent classes when
the class supplied its own __init__ and set (init=False) on the dataclass.

It also fixes a bug with multiple inheritance of dataclasses.

Previously dataclasses attempted to look up the current class' __init__ to find
the definition of InitVars, which worked fine as long as the current class was
a direct subclass of the parent and didn't override __init__.

Unfortunately that didn't work when the InitVar came from a subclass that was
not first in MRO, or the parent had an item in its __init__ definition that
the subclass didn't use.

Now mypy will look up the __init__ for the current parent class that is being
processed in order to find the appropriate InitVar definition.

I've added test cases for all the issues.

This fixes two bugs related to dataclass InitVars and overriding __init__
(python#8022 and python#8015) by changing mypy to ignore InitVars in parent classes when
the class supplied its own __init__ and set (init=False) on the dataclass.

It also fixes a bug with multiple inheritance of dataclasses.

Previously dataclasses attempted to look up the current class' __init__ to find
the definition of InitVars, which worked fine as long as the current class was
a direct subclass of the parent and didn't override __init__.

Unfortunately that didn't work when the InitVar came from a subclass that was
not first in MRO, or the parent had an item in its __init__ definition that
the subclass didn't use.

Now mypy will look up the __init__ for the current parent class that is being
processed in order to find the appropriate InitVar.
Copy link
Member

ilevkivskyi left a comment

Thanks for PR, looks very good! I just have couple small suggestions about tests.

test-data/unit/check-dataclasses.test Outdated Show resolved Hide resolved
test-data/unit/check-dataclasses.test Show resolved Hide resolved
ilevkivskyi merged commit 387a911 into python:master Dec 19, 2019
2 checks passed
beckjake deleted the handle-initvar-overrides branch Dec 23, 2019
beckjake pushed a commit to beckjake/mypy that referenced this issue Dec 27, 2019
In python#8159, I fixed InitVar handling so that it looked up the superclass __init__ and used that to determine the definition of InitVars.
That fix doesn't work in the presence of `init=False` subclasses of the initial `init=False` class
In that case, the lookup fails because the first attribute reached in MRO order on the subclass is the intermediate class, which also doesn't have the variable in its `__init__`.
Then mypy didn't know about the InitVar but still tried to process it as if it did, resulting in an assertion error.

This PR fixes that issue by not adding the field to the set of known attrs until an actual definition is found.
JukkaL pushed a commit that referenced this issue Jan 9, 2020
…8208)

Fixes #8207

In #8159, I fixed InitVar handling so that it looked up the superclass __init__ 
and used that to determine the definition of InitVars. That fix doesn't work 
in the presence of `init=False` subclasses of the initial `init=False` class
In that case, the lookup fails because the first attribute reached in MRO 
order on the subclass is the intermediate class, which also doesn't have 
the variable in its `__init__`. Then mypy didn't know about the InitVar but 
still tried to process it as if it did, resulting in an assertion error.

This PR fixes that issue by not adding the field to the set of known attrs 
until an actual definition is found.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants