Repository navigation
Replace # type: ignores without error codes #7497
Description
pyright recently added # pyright: ignore[...]. We should replace all plain # type: ignores with either # type: ignore[...] or # pyright: ignore[...] if possible. There might be a few ignores we can't replace.
$ rgrep "# type: ignore" stubs/ stdlib/ | grep -v "# type: ignore\[" | grep -v @python2 | wc -l
101
Reacted by Alex Waygood and Mehdi Drissi
Activity
srittau commentedon Mar 16, 2022
Cf #7495
AlexWaygood commentedon Mar 16, 2022
If we do this for all the
type: ignores that are irrelevant to mypy, we can also consider trying out the newignore-without-codemypy option, which will complain if we have atype: ignorecomment without a mypy error code.JelleZijlstra commentedon Mar 16, 2022
Some ignores may also be necessary for pytype (and maybe pyre, which we don't run in CI).
rchen152 commentedon Mar 16, 2022
Pytype discards
# type: ignores without doing anything with them, so removing them shouldn't affect it :)Akuli commentedon Mar 16, 2022
Let's say someone makes a new type checker that doesn't attempt to parse
# pyright: ignore[...]comments, but understands the standard# type: ignorecomments. Should it display an error if it notices that there's something wrong with the stubs? Or just silence all errors coming from stub files?I generally don't like introducing type checker specific things into stubs. While
# type: ignore[error-code]is mypy-specific, other type checkers tend to treat it the same as# type: ignore. But how is a type checker supposed to know what to do with# pyright: ignore[...]?I find it unlikely that we need to say "ignore this error in pyright but not in any other type checker". An error like that would be basically a bug in pyright, and those get fixed very quickly.
srittau commentedon Mar 17, 2022
The ignores serve our internal tests. Type checkers shouldn't warn about perceived problems in typeshed or external stubs. (I also think it's also problematic for a type checker to treat "malformed" ignores as a regular ignore.) Everything else would mean we'd potentially have to annotate every line in typeshed with a ignore, because who knows what a type checkers decides to warn about.
Using plain
# type: ignorein stubs is dangerous as it hides potential problems. It's also a documentation issue as it's often not immediately clear why a certain line is ignored.srittau commentedon Mar 17, 2022
One further advantage of using error codes is that it gives us the chance to collect statistics about the types of
# type: ignores we use. This could influence the options we use in tests and point to areas of improvement in type checkers.AlexWaygood commentedon Mar 17, 2022
Having filed #7504, I can report that the
type: ignores needed by pyright but not mypy are these, these and these. There appear to be three kinds of error that (with our current CI) pyright picks up on where mypy doesn't:self/classmethods where the first parameter isn't namedcls.TypeVaronly once, leaving theTypeVarunbound.There appear to be only 12
type: ignores across the codebase that are needed by pyright but not by other type checkers, following the changes made to the pyright config settings in #7498.srittau commentedon Mar 17, 2022
Point 3 is a warning we should probably just disable in typeshed. Point 1 could be disabled if stubtest checks the name of
self/clsagainst the implementation. Point 2 is very useful, though, and we should probably review those cases.AlexWaygood commentedon Mar 17, 2022
Here's the rundown of what stubtest checks for us:
self/clsfor dunders. (In fact, it doesn't check any parameter names for dunders. And there are many which are incorrect.)__init__and__new__, and it does check all parameter names for non-dunder methods.So, there are a few holes in stubtest's checks with regards to parameter names, and it's possible that pyright could catch a few typos.
It's also equally possible that this pyright check is making it more difficult to match the runtime implementation, though. Most dunders in the
fractionsstdlib module don't haveselfas their first argument name. If we wanted to match that in the typeshed stubs, we'd currently have to litter the module withtype: ignores.It also probably just doesn't really matter that much to match the parameter names in dunders, though. So I don't think it's a massive problem keeping the pyright config as it is for point 1.
Use error codes for type ignores
Use error codes for type ignores (#8280)