-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Remove redundant reportUnnecessaryTypeIgnoreComment=true
#9622
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
Remove redundant reportUnnecessaryTypeIgnoreComment=true
#9622
Conversation
Are the pyright configs used when checking scripts? As far as I understand, our scripts are currently checked with mypy only, at least in CI. That said, I'd be glad if we could apply this PR. |
Cc @AlexWaygood |
The pyright configs are not used when checking our scripts, but they are used when checking our test-case files. The stdlib test cases have a dedicated pyrightconfig file all to themselves, but the third-party test cases use the same config files as our third-party stubs (they have to, because the third-party test cases are in the same top-level folder as the third-party stubs). The reason why I initially introduced this check into check_consistent.py was because we didn't at the time have Since, as @Avasam says, Line 36 in b5a26d1
Presumably pyright won't emit any errors for unused |
Pyright is run on the
@AlexWaygood typeshed/pyrightconfig.testcases.json Lines 8 to 10 in b748a9a
|
My suspicions are confirmed — pyright should be emitting an error on the first commit of this PR about the unused
Remember that we don't use this script in CI; we use
As I explained above, the |
Okay, so we can see in #9623 that pyright isn't verifying our third-party test cases guarding against false negatives in our CI. I also can't work out how to fix that. Adding The good news is that pyright is verifying our stdlib test cases guarding against false negatives: see #9624. |
Thank you for raising this issue! I'm closing this in favour of #9650, but I've listed you as a co-author over there :) |
Seems pretty redundant since #9397
reportUnnecessaryTypeIgnoreComment
is set toerror
in all configs