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

Support if statements in dataclass_transform class #14854

Merged
merged 6 commits into from
Mar 9, 2023
Merged

Support if statements in dataclass_transform class #14854

merged 6 commits into from
Mar 9, 2023

Conversation

KRunchPL
Copy link
Contributor

@KRunchPL KRunchPL commented Mar 8, 2023

Fixes: #14853

Adding support for if statements in the dataclass_transform class, so the attributes defined conditionally are treated as those that are directly in class body.

@github-actions

This comment has been minimized.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks! The usecase with if TYPE_CHECKING seems reasonable to me.
I personally use this from time to time.

mypy/plugins/dataclasses.py Outdated Show resolved Hide resolved
mypy/plugins/dataclasses.py Outdated Show resolved Hide resolved
mypy/plugins/dataclasses.py Outdated Show resolved Hide resolved
present_2: int
if not False:
present_3: int
if not TRUTH:
Copy link
Member

Choose a reason for hiding this comment

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

Let's add at least a single test with some def cond() -> bool: ... function.

if cond():
   x: int
   y: int
   z1: int
else:
   x: str
   y: int
   z2: int

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added the test. It is of course expecting errors, because mypy cannot know what is the return value of a function right? The condition logic is based on IfStmt.is_unreachable which is filled in by logic found in mypy.reachability. I have added a test with some comments that not only tests, but also shows the behavior. I am not sure if we can avoid this limitation (although I am new here, so I can be mistaken). And of course most typical use case will be if TYPE_CHECKING, which we can know is ALWAYS_TRUE.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think the test is correct 👍

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

One last thing from me: as far as I understand - this does also affect how @dataclasses are defined.

Can we please add a single test for it as well?

Thanks a lot for your work! 👍

@github-actions

This comment has been minimized.

@KRunchPL
Copy link
Contributor Author

KRunchPL commented Mar 8, 2023

Added the test for pute dataclass.

@KRunchPL KRunchPL requested a review from sobolevn March 8, 2023 20:05
@github-actions
Copy link
Contributor

github-actions bot commented Mar 8, 2023

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@sobolevn sobolevn merged commit 2e8dcbf into python:master Mar 9, 2023
@sobolevn
Copy link
Member

sobolevn commented Mar 9, 2023

Thank you!

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

Successfully merging this pull request may close these issues.

When using dataclass_transform, __init__ does not know about attributes defined in TYPE_CHECKING
2 participants