-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Make disallow-any-unimported
flag invertible
#18030
Make disallow-any-unimported
flag invertible
#18030
Conversation
@@ -677,6 +671,12 @@ def add_invertible_flag( | |||
help="Disallow usage of generic types that do not specify explicit type parameters", | |||
group=disallow_any_group, | |||
) | |||
add_invertible_flag( |
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.
I moved this down ~20 lines or so, as all invertible flags for this group were defined after non-invertible flags prior to now and I wanted to maintain that convention (assuming this was intentional and not just a coincidence)
@@ -963,6 +963,12 @@ from missing import Unchecked | |||
|
|||
t: Unchecked = 12 # E: Type of variable becomes "Any" due to an unfollowed import | |||
|
|||
[case testAllowImplicitAnyVariableDefinition] |
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.
I'm not fully sure how to test explicitly setting what is already the default behaviour. This test will pass whether or not --allow-any-unimported
is set (as long as --disallow-any-unimported
is not set).
This is a copy of the test immediately before it. The test before this sets --disallow-any-unimported
and asserts that an error occurs, this test sets --allow-any-unimported
and asserts no error occurs.
ad42848
to
e7d709b
Compare
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
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.
Thanks! The specific change looks good.
That said, I'm obligated to disclaim that global --follow-imports=skip
is evil. It's the most commonly misused mypy config I see. If someone random is reading this, you usually want --disable-error-code import-untyped
or maybe --ignore-missing-imports
Absolutely agree 👍 We have The aim here is to support special cases where it may make sense to do partial type checking of a single modified file locally in a very large project (by using Use of these flags should always be an exception when manually running mypy, and not part of your standard configuration. |
The
--follow-imports=skip
CLI option is pretty much unusable in a project wheredisallow_any_unimported
is set to true in the configuration file, as this results in a large number of errors (due to both flags being incompatible). We have a pretty standard project configuration file (withdisallow_any_unimported = true
andfollow_imports = 'normal'
), but for specific local development cases where we want to run the mypy CLI with--follow-imports=skip
it's incredibly noisy due to the number of errors produced.This change proposes making the
disallow-any-unimported
invertible, so that the CLI can be used with--follow-imports=skip
in a less noisy way by using: