Skip to content

Commit

Permalink
Rewrite mypy precommit hook
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ibratoev committed May 6, 2024
1 parent 3eb26d5 commit 3849127
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 23 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,6 @@ jobs:
pre-commit run --all-files flake8
pre-commit run --all-files prettier
pre-commit run --all-files check-yaml
# mypy issues fixes are in progress so, for now, ignore errors
pre-commit run --all-files mypy || true
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ server.htpasswd
server.authz
server.authn

.venv
.venv*
venv
.env
.chroma
Expand Down
37 changes: 15 additions & 22 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,30 +31,23 @@ repos:
- "--extend-ignore=E203,E501,E503"
- "--max-line-length=88"

- repo: https://github.com/pre-commit/mirrors-mypy
rev: "v1.2.0"
- repo: local
hooks:
- id: mypy
args:
[
--strict,
--ignore-missing-imports,
--follow-imports=silent,
--disable-error-code=type-abstract,
--config-file=./pyproject.toml,
]
additional_dependencies:
[
"types-requests",
"pydantic",
"overrides",
"hypothesis",
"pytest",
"pypika",
"numpy",
"types-protobuf",
"kubernetes",
]
name: mypy
entry: "./bin/run-mypy.sh"
language: python
stages: [manual] # run manually for all issues are fixed
pass_filenames: false
# use your preferred Python version
language_version: python3.12
# trigger for commits changing Python files
types_or: [python, toml]
# use require_serial so that script
# is only called once per commit
require_serial: true
# print the number of files as a sanity-check
verbose: true

- repo: https://github.com/pre-commit/mirrors-prettier
rev: "v3.1.0"
Expand Down
7 changes: 7 additions & 0 deletions bin/run-mypy.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/bin/bash

set -e

pip install -r requirements.txt -r requirements_dev.txt --no-input --quiet

mypy .
5 changes: 5 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ pythonpath = ["."]

[tool.mypy]
ignore_errors = false
disable_error_code = "type-abstract"
ignore_missing_imports = false
strict = false # disable strict mypy checks until we can fix all the errors
exclude = "bin/.*"
plugins = "numpy.typing.mypy_plugin"

[[tool.mypy.overrides]]
module = ["chromadb.proto.*"]
Expand Down
2 changes: 2 additions & 0 deletions requirements_dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ grpcio-tools
httpx
hypothesis<=6.98.9 # > Than this version has API changes we don't currently support
hypothesis[numpy]<=6.98.9
mypy==1.10.0
mypy-protobuf
pre-commit
pytest
pytest-asyncio
setuptools_scm
types-protobuf
types-PyYAML
types-requests==2.30.0.0

0 comments on commit 3849127

Please sign in to comment.