Review/reconsider PC140 precommit + mypy

[Carreau]

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.

[Carreau]

see python/mypy#13916

[henryiii]

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

[Carreau]

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.

[Carreau]

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

[henryiii]

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.

[henryiii]

I found the PR at jupyter/jupyter_client#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.

[henryiii]

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.

[Carreau]

The commit is part of jupyter/jupyter_client#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)