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

Fix mishandling of typing.Self in attrs generated inits #14689

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

davfsa
Copy link
Contributor

@davfsa davfsa commented Feb 13, 2023

Fix mishandling of typing.Self in attrs generated inits by using the same expansion method as the dataclasses plugin

Fixes #14685

@github-actions

This comment has been minimized.

@davfsa
Copy link
Contributor Author

davfsa commented Feb 13, 2023

The test run seems a bit confusing, as locally, it succeeds (outside of the test suite). Is it possible that the test have some configuration that may cause the behaviour to change? (Looking more towards strict_optional)

@A5rocks
Copy link
Contributor

A5rocks commented Feb 13, 2023

You can specify flags at the top of the test (# flags: --strict-optional might work?)

Additionally I think this expand_type function should go into common.py to reduce duplication

@davfsa
Copy link
Contributor Author

davfsa commented Feb 13, 2023

You can specify flags at the top of the test (# flags: --strict-optional might work?)

The flag didn't seem to do much, specially given that it seems to be the default since mypy 0.600 (as per https://mypy.readthedocs.io/en/stable/kinds_of_types.html#no-strict-optional)

This is really confusing, but I will have another look later

@davfsa
Copy link
Contributor Author

davfsa commented Feb 14, 2023

@A5rocks you were actually right with the flag, i just seemed to have typed flag instead of flags, which is why it didnt work. Thanks for pointing me in the right direction!

I also merged similar code function as you pointed out. Hope it looks good :)

@github-actions

This comment has been minimized.

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! A few comments and questions below.

mypy/expandtype.py Outdated Show resolved Hide resolved
mypy/plugins/dataclasses.py Outdated Show resolved Hide resolved
test-data/unit/check-attr.test Outdated Show resolved Hide resolved
@github-actions

This comment has been minimized.

@davfsa
Copy link
Contributor Author

davfsa commented Feb 15, 2023

@JukkaL Addressed one of your points and got questions about the other ones :)

@davfsa davfsa requested review from JukkaL and ilevkivskyi and removed request for JukkaL and ilevkivskyi February 17, 2023 09:20
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2023

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attrs plugin mishandles typing.Self in generated inits
5 participants