-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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
gh-109125: Run mypy on Tools/wasm
#109126
Conversation
python_version = 3.8 | ||
|
||
# Be strict... | ||
strict = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this mean all code in this directory must be fully typed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can ignore any code that is not typed:
- Via adding global
disallow_incomplete_defs = false
- Via per-file ignores
- Via inline
# type: ignore
For now, yes, all code must be typed. Do you want to add disallow_incomplete_defs = false
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to add
disallow_incomplete_defs = false
?
Yes please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing my review to make type checking opt-in per-file, but other LGTM.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
90066fb
to
a1725b4
Compare
Done! I have made the requested changes; please review again |
Thanks for making the requested changes! @brettcannon: please review the changes made to this pull request. |
@brettcannon updated branch to fix freebsd unrelated failire |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And backport to 3.12?
(We don't yet have .github/workflows/mypy.yml
in 3.11.)
Sorry, @sobolevn and @hugovk, I could not cleanly backport this to |
(cherry picked from commit f65497f) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
GH-109561 is a backport of this pull request to the 3.12 branch. |
|
|
|
|
Some comments:
type: ignore
can be removed later, when we add these members totypeshed/
PurePath
toPath
, because they were using.exists()
method, which does not exist onPurePath
and goes againts its purposerun_py
to returnint
, because that's what similar methods doTools/wasm
#109125