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

[stubgen] Improve dataclass init signatures #18430

Merged
merged 1 commit into from
Jan 19, 2025

Conversation

cdce8p
Copy link
Collaborator

@cdce8p cdce8p commented Jan 7, 2025

Remove generated incomplete __init__ signatures for dataclasses. Keep the field specifiers instead.

Copy link
Contributor

github-actions bot commented Jan 7, 2025

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

@hamdanal
Copy link
Collaborator

Thanks I’ll take a look shortly. In the meantime could you check with the typeshed people that this wouldn’t crash their CI? I’m mainly concerned about pytype not accepting assignments in stubs but haven’t checked if it works correctly now. My point is if typeshed is not ready we should wait a bit here before adding this to stubgen.

Copy link
Collaborator

@hamdanal hamdanal left a comment

Choose a reason for hiding this comment

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

Thanks, change looks good. Please check with typeshed maintainers if this is OK before merging.

@cdce8p
Copy link
Collaborator Author

cdce8p commented Jan 19, 2025

In the meantime could you check with the typeshed people that this wouldn’t crash their CI? I’m mainly concerned about pytype not accepting assignments in stubs but haven’t checked if it works correctly now.

To be fair I'm not too familiar with pytype. However, I don't think these assignments should be an issue. Type checkers already need to be able to parse dataclass assignment in .py files, why shouldn't it work in .pyi files then? It works for mypy and pyright without issues.

/CC @AlexWaygood I believe you're one of the typeshed maintainers. Do you know if dataclasses.field assignments in stubs would cause issues?

@cdce8p cdce8p mentioned this pull request Jan 19, 2025
3 tasks
@AlexWaygood
Copy link
Member

Type checkers already need to be able to parse dataclass assignment in .py files, why shouldn't it work in .pyi files then?

I believe pytype uses a different parser for .pyi files rather than .py files; it often crashes on syntax in stubs that mypy and pyright are totally fine with. The way to find out whether it would cause issues for pytype would probably be to open a draft PR to typeshed adding some field() calls as assignment values in some dataclass classes, e.g. this one https://github.com/python/typeshed/blob/c01e731dd1fb6d6ae3c9ef00e2f19a89ab1ffb24/stdlib/pstats.pyi#L48-L61. We can see if pytype crashes on the change.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Seems reasonable, thank you!

@hauntsaninja hauntsaninja merged commit 68cffa7 into python:master Jan 19, 2025
18 checks passed
@cdce8p cdce8p deleted the stubgen-dataclass branch January 19, 2025 21:58
@cdce8p
Copy link
Collaborator Author

cdce8p commented Jan 19, 2025

Just to catch up here. We were just in the process of testing it with typeshed. Seems pytype does indeed have an issue with it, as well as flake-pyi and ruff. I don't think this is a problem though as stubgen is meant to be a starting point to iterate from.

python/typeshed#13415

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

Successfully merging this pull request may close these issues.

4 participants