scientific-python
Review/reconsider PC140 precommit + mypy
I'm in the process of removing/forbiding the run of mypy via pre-commit/hatch on jupyter_client and likely other places as running via pre-commit/hach can (often?) swallow error.
try to clone jupyter client main, currently 0bce36fb216d228d755f6c1ca3473cfb4e0f29a6, and run:
$ hatch run typing:test # noerror
$ pre-commit run --all-files --hook-stage manual mypy #no error
$ mypy # this works and detect **real** issues.
My guess is pre-commit/hatch create envs just to run those tools, and if something goes wrong and mypy run in those envs, it may find the wrong version of your package.
You should always run mypy in a typing environment. Typing dependencies are not runtime dependencies. Sometimes they are, but sometimes they are not. That's not the problem.
Your hatch task literally just runs pre-commit. So it's not anything to do with hatch, you are just running pre-commit inside hatch.
When you are running mypy without pre-commit, you probably forgot to install the dependencies. If I do that, I get lots of errors, but they all come from missing traitlets ipykernel jupyter_core, which are installed by the pre-commit hook, or types-pexpect types-paramiko types-netifaces types-psutil types-python-dateutil, which are installed by --install-types in the pre-commit hook (though you really should list them explicitly and not use that flag, it will cache better). These are not runtime dependencies, by the way.
If I do all that, it passes, just like the pre-commit hook. So then enforces that you should always run it in pre-commit.
$ gh repo clone jupyter/jupyter_client
$ cd jupyter_client
$ uv venv
$ . .venv/bin/activate.fish
$ uv pip install mypy==1.18.2
$ uv pip install traitlets ipykernel jupyter_core
$ uv pip install types-pexpect types-paramiko types-netifaces types-psutil types-python-dateutil
$ mypy
Success: no issues found in 38 source files
I do have those dependencies installed; and I tried to running it directly i hatch as well (with local modifs), I am aware that that hatch calls pre-commit I find it horrendous to nest that many env but anyway, I was not the one who made those choices). Running via pre-commit did miss a typo in a PR (options not option), and I do not really understand why/when pre-commit swallows errors but it sometime (often) does.
I agree that we should run mypy, but enforcing it via pre-commit seem weird.
Here is the specific commit PR where mypy was not failing because of pre-commit:
jupyter_client[8e73d8af] $ pre-commit run --all-files --hook-stage manual mypy
mypy.....................................................................Passed
jupyter_client[8e73d8af] $ mypy
jupyter_client/session.py:133: error: Incompatible types in assignment (expression has type "None", variable has type Module) [assignment]
orjson = None
^~~~
jupyter_client/session.py: note: In function "orjson_packer":
jupyter_client/session.py:142: error: Unexpected keyword argument "options" for "dumps"; did you mean "option"? [call-arg]
return orjson.dumps(obj, default=json_default, options=options)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
jupyter_client/session.py: note: At top level:
jupyter_client/session.py:157: error: Skipping analyzing "msgpack": module is installed, but missing library stubs or py.typed marker
[import-untyped]
import msgpack # type:ignore[import-not-found]
^
jupyter_client/session.py:157: note: Error code "import-untyped" not covered by "type: ignore" comment
jupyter_client/session.py:157: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
We don't "enforce it" via pre-commit, we just have a check for that which you can skip (with a description these days, too!). If we could find it reliable in tox/nox/hatch, then we could also check for that, but I don't know of a good way to do that reliably.
You are missing msgpack locally, does installing that change things? Missing dependencies cause other failures that look unrelated. Is there a way I can find that commit, is there a link to the PR? I don't see orjson in the code. By the way, IIRC mypy runs faster if orson is installed.
I found the PR at https://github.com/jupyter/jupyter_client/pull/1064, pre-commit is working perfectly.
jupyter_client/session.py:131: import orjson # type:ignore[import-not-found]
That import tells mypy to treat orjson as Any if it's not installed. It wasn't added to the pre-commit additional_dependencies, so it is Any, and can't tell that there's a mistake. Same happens locally if you don't install orjson. Don't ever do that - add it to pyproject.toml if you have to, that is, it doesn't support typing and you don't want to write stubs.
I'd love improvements to the experience here, there are some quirks:
- You need to specify
args(even if empty), because the mypy hook has the ignore imports flag (terrible). MyPy could offer their own hook to fix this, I think. - You can't pull dependencies from pyproject.toml; I understand that pre-commit wants to stay general (and the author hates pyproject.toml, and toml in general) - I think prek might fix this eventually though!
- You can't lock anything other than mypy and get auto-updates; another thing I think prek might fix eventually
But you do want a controlled environment for mypy, it's not something you should run in a normal environment.
That said, I use mypy in pre-commit in almost every project I work on, including packaging, pybind11, scikit-build-core, cibuildwheel, pipx, check-sdist, nox, boost-histogram, hist, flake8-errmsg, plumbum, and others, and it works perfectly and reliably, never missing anything that a manual run would not.
The commit is part of https://github.com/jupyter/jupyter_client/pull/1064 If you think there are really no issues, feel free to close. It's just not the first time for me that I end up with issue with mypy + precommit (and yes it seems to works great like 99% of the time)