mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Cannot suppress python_version-related errors in third-party code

Open kormat opened this issue 3 years ago • 11 comments

Bug Report

If i have python_version = 3.5 set, it's not possible to tell mypy to ignore related syntax errors it finds in dependencies.

Use case: i have code which needs to support python 3.5, as well as newer versions. I run mypy against it under various versions of python, with python_version = 3.5 set so that i don't accidentally introduce something which is incompatible (like variable annotations) which might not get caught until later. This has worked fine until some of the dependencies have started using variable annotations.

To Reproduce

  1. Create a venv using python 3.6+.
  2. pip install mypy importlib-metadata # Using importlib-metadata as an example lib which uses variable annotations
  3. Create setup.cfg with:
[mypy]
python_version = 3.5

[mypy-importlib_metadata]
ignore_errors = True
  1. Create test.py with:
import importlib_metadata
  1. Run mypy test.py

Expected Behavior

mypy should enforce python_version = 3.5 for test.py, but it should ignore any version-related issues in importlib_metadata.

Actual Behavior

mypy fails with:

venv/lib/python3.8/site-packages/importlib_metadata/__init__.py:88: error: Variable annotation syntax is only supported in Python 3.6 and greater
Found 1 error in 1 file (errors prevented further checking)

mypy doesn't seem to treat this as a 'fatal' error, either, as setting pdb = True has no effect.

Your Environment

  • mypy 0.790 and 0.800
  • python 3.6.7, 3.6.12 and 3.8.5
  • Debian Stretch and Linux Mint 20.1 (Equivalent to Ubuntu 20.04)

kormat avatar Jan 26 '21 14:01 kormat

I have a hard time understanding how this is a bug. With python_version = 3.5, mypy checks whether your code works in 3.5. You import a package that doesn't work in 3.5, so you get an error.

JelleZijlstra avatar Jan 26 '21 15:01 JelleZijlstra

@JelleZijlstra: if it's not a bug, then i'm not sure how the python_version feature is useful.

The default mypy settings ignore errors in site-packages. The fact that python_version doesn't follow that approach (and this can't be worked-around either) means i can no longer use it, which is a shame.

If that's Working As Intended, then yeah, we might as well just close this.

kormat avatar Jan 27 '21 12:01 kormat

I see, I guess the issue may be that the error you see is classified as a blocking error, and blocking errors in site-packages are not ignored.

JelleZijlstra avatar Jan 27 '21 13:01 JelleZijlstra

I think this might be the same or strictly related to #9981

volans- avatar Jan 27 '21 19:01 volans-

@JelleZijlstra: yeah that sounds right.

@volans-: I don't think this is related, as it also happens with older versions of mypy.

kormat avatar Feb 09 '21 09:02 kormat

I think this is the same issue as https://github.com/python/mypy/issues/14373, but there I was able to resolve it with a module-specific override:

  [[tool.mypy.overrides]]
  module = 'pytest'
  follow_imports = 'skip'

python_version does ignore errors in site packages, unless some package of yours imports them, or some package that you import imports them. In my case, it was pytest importing numpy conditionally, but I don't use any of the features of pytest that use numpy, so I had to add this override.

RE: @JelleZijlstra:

I have a hard time understanding how this is a bug. With python_version = 3.5, mypy checks whether your code works in 3.5. You import a package that doesn't work in 3.5, so you get an error.

I do think it's going to be harder to avoid this type of error as more projects ship with newer type info. I don't think I should have to add exclusions for (potentially) every conditional import in my transitive dependencies, just to allow developers working in newer python versions to run mypy.

It would be nice if there were some follow_imports mode that would include anything my project imports directly, but not transitive imports (as these may be conditional). I gave an example in https://github.com/python/mypy/issues/14373#issuecomment-1368052759. Is that possible/reasonable?

tgamblin avatar Dec 31 '22 21:12 tgamblin

@tgamblin I think you can just skip numpy alone (so you don't need to figure out what's importing it), see https://github.com/python/mypy/issues/14373#issuecomment-1368294473 . Also I think master mypy is better at skipping analysis of things it might not need, so e.g. mypy -c 'import pytest' just works for me on master.

hauntsaninja avatar Dec 31 '22 23:12 hauntsaninja

@hauntsaninja I wasn't able to skip numpy alone to make this work:

> grep -A 4 'with numpy' pyproject.toml
  # fix python version errors with numpy
  [[tool.mypy.overrides]]
  module = 'numpy'
  follow_imports = 'skip'
  follow_imports_for_stubs = false
> spack style --tool mypy
==> Running style checks on spack
  selected: mypy
==> Running mypy checks
var/spack/environments/default/.spack-env/._view/4g7jd4ibkg4gopv4rosq3kn2vsxrxm2f/lib/python3.11/site-packages/numpy/__init__.pyi:638: error: Positional-only parameters are only supported in Python 3.8 and greater  [syntax]
Found 1 error in 1 file (errors prevented further checking)
  mypy found errors
==> Error: spack style found errors

vs.

> grep -A 4 'with numpy' pyproject.toml
  # fix python version errors with numpy
  [[tool.mypy.overrides]]
  module = 'pytest'
  follow_imports = 'skip'
  follow_imports_for_stubs = false
> spack style --tool mypy
==> Running style checks on spack
  selected: mypy
==> Running mypy checks
Success: no issues found in 578 source files
  mypy checks were clean
==> spack style checks were clean

This is with 0.991, but I haven't tried master yet.

tgamblin avatar Jan 01 '23 00:01 tgamblin

You need follow_imports_for_stubs = true, not false :-)

hauntsaninja avatar Jan 01 '23 00:01 hauntsaninja

Ok that does work! The sense of those options is... kind of confusing. So follow_imports = 'skip' means "don't follow imports of this package, but if it has stubs go ahead", and follow_imports_for_stubs means "the follow_imports option ALSO applies to stubs"? Do I have that right?

I think this is way better, as I only have to know that numpy is too new, not what imports numpy. It's still a bit unsatisfying as eventually there are going to be a lot of libraries that ship stubs, and they may all be newer than what dependent projects support. Project maintainers will still have to except all of them to use python_version.

Are there some best practices for when you should use python_version? We are having a debate about this over on https://github.com/spack/spack/pull/34732.

tgamblin avatar Jan 01 '23 00:01 tgamblin

Yup, you have that right :-) (I think the reason mypy has these multiple options is it made more sense when setting them globally. I agree that it's non-obvious that you also may have to set follow_imports_for_stubs when trying to set follow_imports for a specific module)

I typically set python_version in my projects, setting it to the oldest Python version the project targets. It helps me catch errors sooner and it makes type checking more reproducible across various contributors' environments. (For example, in your case it'd be reasonable for a contributor to dev locally with Python 3.7, and then they might be surprised at the error you ran into). That said, I think it's rare for it to catch errors that a proper unit test matrix across Python versions wouldn't catch, so if python_version doesn't work for you, I wouldn't sweat it the most.

If it's any comfort, in my experience, it's rare for users to run into the issue you ran into. If you do need something more foolproof, I'd run mypy with Python 3.7. This would avoid the error you ran into, because you'd backsolve to an older numpy that supported Python 3.7 and then not have this issue.

hauntsaninja avatar Jan 01 '23 00:01 hauntsaninja

I do not believe skipping imports is a solution. You could be using the 3rd party module API incorrectly and mypy would no longer flag it up, right?

MetRonnie avatar Jan 18 '24 17:01 MetRonnie