Skip to content

Add basic implementation for mypy: ignore comments - #22180

Open
ilevkivskyi wants to merge 3 commits into
python:masterfrom
ilevkivskyi:mypy-ignore
Open

ilevkivskyi wants to merge 3 commits into
python:masterfrom
ilevkivskyi:mypy-ignore

Conversation

@ilevkivskyi

Copy link
Copy Markdown
Member

Fixes #12358

Implementation is mostly straightforward, but there are few things:

  • This requires Straighten type comment parsing mypyc/ast_serialize#95, so tests will fail until we merge that.
  • We forgot to bump cache version to 6. This is trivial however, functions now have a flag indicating if they have a type comment.
  • I found a pretty bad bug in native parser: parse errors cannot be type-ignored even if they are non-blocking. Again, it is a one line change, so not making a separate PR for this.

Finally, there is one thing that bothers me a bit: we have few places where we mention words type: ignore in errors and notes. However, it would take a significant effort to reliably change them to mypy: ignore when needed. Essentially we would need to record provenance for each error code, and thread it through to many places (including to cache metas, for some technical reasons). So we have three options:

  1. Do nothing about it. mypy: ignore really means mypy-specific type ignore, so it is fine to use type: ignore in error notes, if we actually have mypy: ignore written in code.
  2. Actually implement all the work needed (not a fan of this option TBH).
  3. Change the wording in those few errors/notes so that it is more neutral: i.e. say Unused "ignore" comment instead of Unused "type: ignore" comment etc.

I am leaning towards option 3, but wanted to double-check before actually doing this.

cc @JukkaL

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

comtypes (https://github.com/enthought/comtypes)
+ comtypes/_post_coinit/misc.py:27: error: Unused "type: ignore" comment  [unused-ignore]
+ comtypes/__init__.py:26: error: Unused "type: ignore" comment  [unused-ignore]
+ comtypes/__init__.py:34: error: Unused "type: ignore" comment  [unused-ignore]
+ comtypes/connectionpoints.py:10: error: Unused "type: ignore" comment  [unused-ignore]

pydantic (https://github.com/pydantic/pydantic)
- pydantic/_internal/_utils.py:347: error: "Obj" has no attribute "copy"  [attr-defined]

rotki (https://github.com/rotki/rotki)
+ rotkehlchen/chain/ethereum/modules/eth2/eth2.py:681: error: Unused "type: ignore" comment  [unused-ignore]

pandas-stubs (https://github.com/pandas-dev/pandas-stubs)
+ tests/test_windowing.py:366: note: Error code "syntax" not covered by "type: ignore[arg-type]" comment

xarray (https://github.com/pydata/xarray)
+ xarray/tests/test_array_api.py:11: error: Unused "type: ignore" comment  [unused-ignore]

sockeye (https://github.com/awslabs/sockeye)
+ sockeye/layers.py:642: error: Unused "type: ignore" comment  [unused-ignore]

scipy-stubs (https://github.com/scipy/scipy-stubs)
- tests/misc/test_submodules_empty.pyi:3: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports

@ilevkivskyi

Copy link
Copy Markdown
Member Author

Primer looks good. All changes look like a change towards (most likely) desired behavior. In particular, there are few cases like this:

    some(code)  # mypy can't handle this  # type: ignore[arg-type]

while in fact mypy can now handle this, so the ignore is unused (and zero cases of the opposite, i.e. # avoid another # type: ignore).

@A5rocks A5rocks left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Basically I just have a couple nitpicks.

Comment thread mypy/nativeparse.py
{
"line": line,
"column": -1,
"message": f'Unrecognized error code "{c}"',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know the old code had the same thing, but I think this should have the original prefix, i.e. if I get an error for # type: ignore[mypy:fake] I expect to get Unrecognized error code "mypy:fake".

Comment thread mypy/nativeparse.py
{
"line": line,
"column": -1,
"message": 'Error codes in "mypy: ignore" should not be prefixed',

@A5rocks A5rocks Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

IMO we should be explicit about the prefix. I for one didn't know you could prefix error codes! i.e. Error codes in "mypy: ignore" should not be prefixed with "mypy:".

Comment thread mypy/nativeparse.py
for line in sorted(ignores_by_line.keys() | mypy_ignores_by_line.keys()):
codes = ignores_by_line.get(line)
mypy_codes = mypy_ignores_by_line.get(line)
if codes is not None and mypy_codes is None:

@A5rocks A5rocks Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry, I don't really know the parser and taking a look didn't really help. Does None mean # type: ignore? If so, I imagine the following blanket case should also apply here!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support mypy: ignore inline comments to suppress mypy errors

2 participants