Skip to content
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

Cannot suppress python_version-related errors in third-party code #9972

Open
kormat opened this issue Jan 26, 2021 · 12 comments
Open

Cannot suppress python_version-related errors in third-party code #9972

kormat opened this issue Jan 26, 2021 · 12 comments
Labels
bug mypy got something wrong

Comments

@kormat
Copy link

kormat commented Jan 26, 2021

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 kormat added the bug mypy got something wrong label Jan 26, 2021
@JelleZijlstra
Copy link
Member

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.

@kormat
Copy link
Author

kormat commented Jan 27, 2021

@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.

@JelleZijlstra
Copy link
Member

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.

@volans-

This comment has been minimized.

@kormat
Copy link
Author

kormat commented Feb 9, 2021

@JelleZijlstra: yeah that sounds right.

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

@tgamblin
Copy link

tgamblin commented Dec 31, 2022

I think this is the same issue as #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 #14373 (comment). Is that possible/reasonable?

@hauntsaninja
Copy link
Collaborator

@tgamblin I think you can just skip numpy alone (so you don't need to figure out what's importing it), see #14373 (comment) . 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.

@tgamblin
Copy link

tgamblin commented Jan 1, 2023

@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.

@hauntsaninja
Copy link
Collaborator

You need follow_imports_for_stubs = true, not false :-)

@tgamblin
Copy link

tgamblin commented Jan 1, 2023

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 spack/spack#34732.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Jan 1, 2023

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.

wmfphab pushed a commit to wikimedia/operations-software-wmfmariadbpy that referenced this issue Jan 13, 2024
Patch Set 2:

I've filed python/mypy#9972 with upstream.

Patch-set: 2
@MetRonnie
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

No branches or pull requests

6 participants