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

Using mypy via pre-commit #13916

Closed
hauntsaninja opened this issue Oct 17, 2022 · 0 comments
Closed

Using mypy via pre-commit #13916

hauntsaninja opened this issue Oct 17, 2022 · 0 comments
Labels
bug mypy got something wrong

Comments

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Oct 17, 2022

pre-commit is useful; many mypy users use mypy via pre-commit. This issue exists so I can link to it when users run into common gotchas. Please do not discuss pre-commit related feature requests here, open a new issue.

pre-commit runs mypy in an isolated environment

pre-commit wants checks to run in isolated environments. mypy wants to know as much about your program as it can, including everything about everything you import. These are in conflict.

The pre-commit mypy mirror thing automatically (and to most users, surprisingly) passes --ignore-missing-imports, which means that most types originating from third party dependencies will become Any: https://github.com/pre-commit/mirrors-mypy/blob/33f4a30be4e66513c51a857712fc732e1a9ddd78/.pre-commit-hooks.yaml#L7
In my opinion, this is bad: it will silently make type checking far less effective. It also means you'll likely get different results when running mypy directly.

Two possible pre-commit fixes. In your pre-commit config 1) Use language: system (and maybe a helper script) to run something from outside a pre-commit environment, 2) Repeat your list of dependencies and stub file dependencies in additional_dependencies

pre-commit passes only diffed files

This is sort of the core idea of pre-commit and for most pre-commit checks, this is great. But mypy really wants to do whole program analysis, and will try to follow all imports, so pre-commit is not actually saving you any work. In fact, if it is saving you work, it's probably undesirable and some bug in your mypy setup. Anyway, this is another source of potential differences between running mypy directly, e.g. there are undesirable interactions with --exclude: mypy assumes that if you pass a file in the command, you want it to be checked.

Recommended pre-commit fix. In your pre-commit config, use pass_filenames: false

@hauntsaninja hauntsaninja added the bug mypy got something wrong label Oct 17, 2022
@hauntsaninja hauntsaninja closed this as not planned Won't fix, can't repro, duplicate, stale Oct 17, 2022
rababerladuseladim added a commit to robert-koch-institut/mex-backend that referenced this issue Nov 30, 2023
- mypy pre-commit hook now always runs on all files instead of only
changed files, which is the python/mypy#13916
- mypy now shows errors in case of missing imports (prior: silently
ignored missing imports)
- move content of .mypy.ini and pytest.ini into pyproject.toml
rababerladuseladim added a commit to robert-koch-institut/mex-common that referenced this issue Nov 30, 2023
… pytest and mypy config into pyproject.toml (#50)

- mypy pre-commit hook now always runs on all files instead of only
changed files, which is the [recommended way according to the mypy
maintainer](python/mypy#13916)
- mypy now shows errors in case of missing imports (prior: silently
ignored missing imports)
- move content of `.mypy.ini` and `pytest.ini` into pyproject.toml
- update dependencies
rababerladuseladim added a commit to robert-koch-institut/mex-drop that referenced this issue Nov 30, 2023
- mypy pre-commit hook now always runs on all files instead of only
changed files, which is the python/mypy#13916
- mypy now shows errors in case of missing imports (prior: silently
ignored missing imports)
- move content of .mypy.ini and pytest.ini into pyproject.toml
carolinscholl pushed a commit to robert-koch-institut/mex-common that referenced this issue Dec 6, 2023
… pytest and mypy config into pyproject.toml (#50)

- mypy pre-commit hook now always runs on all files instead of only
changed files, which is the [recommended way according to the mypy
maintainer](python/mypy#13916)
- mypy now shows errors in case of missing imports (prior: silently
ignored missing imports)
- move content of `.mypy.ini` and `pytest.ini` into pyproject.toml
- update dependencies
ibratoev added a commit to ibratoev/chroma that referenced this issue May 6, 2024
The existing `pre-commit/mirrors-mypy` works by checking individual files and ignoring imports. This is against the whole idea of mypy and limits its benefits. Reimplement the hook to run mypy on all the files in an isolated environment. More on the topic:
* python/mypy#13916
* https://jaredkhan.com/blog/mypy-pre-commit

Additionally:
* Move all the mypy config to pyproject.toml.
* Lock the version of mypy.
* Add types-PyYAML to dev requirements.
* Ignore all `.venv*` folders to support using multiple venvs for different python versions.
* Run pre-commit mypy during PR lint CI and ignore errors as fixes are in progress.
* Disable the mypy pre-commit hook until it is fixed.
ibratoev added a commit to ibratoev/chroma that referenced this issue May 6, 2024
The existing `pre-commit/mirrors-mypy` works by checking individual files and ignoring imports. This is against the whole idea of mypy and limits its benefits. Reimplement the hook to run mypy on all the files in an isolated environment. More on the topic:
* python/mypy#13916
* https://jaredkhan.com/blog/mypy-pre-commit

Additionally:
* Move all the mypy config to pyproject.toml.
* Lock the version of mypy.
* Add types-PyYAML to dev requirements.
* Ignore all `.venv*` folders to support using multiple venvs for different python versions.
* Run pre-commit mypy during PR lint CI and ignore errors as fixes are in progress.
* Disable the mypy pre-commit hook until it is fixed.
* Add __init__.py to the examples/chat_with_your_documents folder to make it a package for mypy.
kbsriram added a commit to kbsriram/Adafruit_CircuitPython_PIOASM that referenced this issue May 29, 2024
Barker404 added a commit to Barker404/wordall that referenced this issue Sep 20, 2024
The way that pre-commit installs tools via git tags and ignores the
environment in use is neat but kind of annoying, since it means either
duplicating them, only using the tool via pre-commit, or bypassing the
typical setup with a "system" pre-commit configuration. For ruff I've
removed it from the venv and can only use it via pre-commit, which seems
fine since I only use it in simple ways, but could become annoying
later.

In the case of mypy I immediately ran into a problem with the fact that
I would need to specify my dependencies (or type stubs for them) in the
pre-commit configuration to get it to work correctly in the isolated
environment. I absolutely don't want that duplication, so instead I'm
using the "system" workaround, which isn't much extra configuration.
This follows the logic described here:
python/mypy#13916. I also disabled
pass_filenames due to it not making much sense with mypy, as described
there.

Another pain point is needing to have pre-commit available to commit,
which right now means activating the appropriate environment. Probably a
case where pipx would be ideal, but I don't feel like setting it up
right now.
jonathanperret added a commit to jonathanperret/ayab-desktop that referenced this issue Sep 25, 2024
`pre-commit` will by default create an isolated virtualenv
to run hooks such as `mypy`. This creates well-known issues
because `mypy` won't have access to packages the code depends
on. See python/mypy#13916 for
recommendations by the `mypy` maintainer.

By running `mypy` as a `system` hook as suggested, it will see the exact
same packages as running `mypy` standalone (or through an IDE) will,
which avoids discrepancies that appear only at commit time.

The compromise is that running `mypy` through `pre-commit` will require
a properly initialized virtualenv. However because we add a `files:`
filter to the `mypy` hook, `pre-commit` will not run if no Python
file has been changed; and if Python files have been changed it seems
reasonable to expect a proper development environment has been set up.

Also following the recommendation we set `pass_filenames: false` so that
`pre-commit` does not pass only the changed files to `mypy`.
jonathanperret added a commit to jonathanperret/ayab-desktop that referenced this issue Sep 25, 2024
`pre-commit` will by default create an isolated virtualenv
to run hooks such as `mypy`. This creates well-known issues
because `mypy` won't have access to packages the code depends
on. See python/mypy#13916 for
recommendations by the `mypy` maintainer.

By running `mypy` as a `system` hook as suggested, it will see the exact
same packages as running `mypy` standalone (or through an IDE) will,
which avoids discrepancies that appear only at commit time.

The compromise is that running `mypy` through `pre-commit` will require
a properly initialized virtualenv. However because we add a `files:`
filter to the `mypy` hook, `pre-commit` will not run if no Python
file has been changed; and if Python files have been changed it seems
reasonable to expect a proper development environment has been set up.

Also following the recommendation we set `pass_filenames: false` so that
`pre-commit` does not pass only the changed files to `mypy`.
jonathanperret added a commit to jonathanperret/ayab-desktop that referenced this issue Sep 30, 2024
`pre-commit` will by default create an isolated virtualenv
to run hooks such as `mypy`. This creates well-known issues
because `mypy` won't have access to packages the code depends
on. See python/mypy#13916 for
recommendations by the `mypy` maintainer.

By running `mypy` as a `system` hook as suggested, it will see the exact
same packages as running `mypy` standalone (or through an IDE) will,
which avoids discrepancies that appear only at commit time.

The compromise is that running `mypy` through `pre-commit` will require
a properly initialized virtualenv. However because we add a `files:`
filter to the `mypy` hook, `pre-commit` will not run if no Python
file has been changed; and if Python files have been changed it seems
reasonable to expect a proper development environment has been set up.

Also following the recommendation we set `pass_filenames: false` so that
`pre-commit` does not pass only the changed files to `mypy`.
craigds added a commit to koordinates/kart that referenced this issue Nov 9, 2024
Follows recommendations at python/mypy#13916
craigds added a commit to koordinates/kart that referenced this issue Nov 10, 2024
Follows recommendations at python/mypy#13916
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

1 participant