CI/TYP: run pyright at manual stage in pre-commit #43799
Conversation
|
we cannot bump this now. needs to be a while. (you could do it for the pyright checking i suppose) but that might be complicated. |
Definitly not for a release but not even for development?
Pre-commit allows specifying additional dependencies for a hook, but it seems they need to be from the same language as the hook (pyright: node, numpy: python). I think the best options is to run pyright at the "manual" stage to avoid annoying contributors. This would still make it possible to easily run pyright. |
twoertwein
changed the title
|
Mypy also needs 1.21.0, otherwise it reports many errors. |
.github/workflows/ci.yml
Outdated
| @@ -32,6 +32,9 @@ jobs: | |||
| with: | |||
| fetch-depth: 0 | |||
|
|
|||
| - name: numpy>=1.21.0 for type checkers | |||
There was a problem hiding this comment.
hmm this is very fragile if we change this
There was a problem hiding this comment.
Adding on to this. Can we just make two conda envs in the CI, one for typing and one for everything else?
There was a problem hiding this comment.
Currently, we don't need this to force numpy>1.21.0 because pip/conda already pick numpy 1.21.*.
We could simply remove this change. If at some point the CI picks a too old numpy, we will know very quickly.
There was a problem hiding this comment.
It is probably best to decouple the CI-change from the pre-commit change. I will remove the CI-changes from this PR.
There was a problem hiding this comment.
if you saw #43804 numpy gets installed <1.21 since that is a numba requirement when resolving dependencies.
There was a problem hiding this comment.
if you saw #43804 numpy gets installed <1.21 since that is a numba requirement when resolving dependencies.
Yes, that is why I tried to add this line, but it seems the CI prefers an older numba in favor of a newer numpy :)
There was a problem hiding this comment.
Just to be clear, the goal of this PR has changed from "having numpy>1.21. on the CI and for pre-commit" to "not having pyright as a mandatory part of pre-commit".
| A recent version of ``numpy`` (>=1.21.0) is required for type validation. | ||
|
|
There was a problem hiding this comment.
Does the [email protected] dependency include this? Because if not, as it's running as a local hook, then it won't have access to numpy, even if you have it installed in your virtual environment
Perhaps it should be repo: system?
(granted, I haven't tried running this, am just pointing out a common misconception about repo: local)
There was a problem hiding this comment.
It seems that pre-commit does not isolate pyright from the current python environment: if numpy is installed in the currently active environment, pyright seems to have access to it.
I tried adding it as a dependency (in the pre-commit), but it seems that pre-commit does not support mixing languages (pyright: node, numpy:python) - it tries to use node to install numpy.
There was a problem hiding this comment.
It seems that pre-commit does not isolate pyright from the current python environment: if numpy is installed in the currently active environment, pyright seems to have access to it.
I find this surprising, but if that's the case, then great 👍
|
thanks @twoertwein yep having to manualish now is fine |
with numpy<1.21.0 pyright (and mypy?) cannot determine some types which leads to overlapping overloads. See #43747 (comment)
@attack68