CI: use github action for pyright #47067
Conversation
| @@ -100,6 +91,12 @@ jobs: | |||
| run: ci/code_checks.sh typing | |||
| if: ${{ steps.build.outcome == 'success' }} | |||
|
|
|||
| - uses: jakebailey/pyright-action@v1 | |||
| with: | |||
| # note: keep version in sync with .pre-commit-config.yaml | |||
There was a problem hiding this comment.
Curious, why do we need this in pre-commit-config and here i.e. can this just be run in pre-commit?
There was a problem hiding this comment.
pyright is only run by pre-commit during the manual stage. The idea is to give developers an easy way to run pyright without having to set it up (it depends on node). The CI doesn't run the manual stage.
I think the reason to keep mypy and pyright separate was that otherwise all the typing packages would need to be installed for the pre-commit hook. Maybe there was a better reason :)
There was a problem hiding this comment.
the reason for not running pyright&mypy (by default) in pre-commit is that outdated/missing packages (e.g., numpy) would cause pyright&mypy to fail. We often have issues between numba requiring an older version of numpy but for typing we want the latest numpy.
There was a problem hiding this comment.
@Dr-Irv I think you set the pyright github action for https://github.com/pandas-dev/pandas-stubs up. How did you manage that pyright finds the virtual env? Running pyright directly (instead of using the github action) seemed to have no issue finding the virtual env.
There was a problem hiding this comment.
Make sense for typing checks in pre-commit to not necessarily run on every commit and therefore in manual mode. Is it possible to have the pre-commit job run these pyright checks then in the CI? My motivation is to see if there can be one clear way to run these checks.
There was a problem hiding this comment.
It would be great to only use pre-commit and make mypy&pyright optional!
I think one issue with mypy+pre-commit is that pre-commit isolates the python environment (can probably be avoided by language: system)? Pyright doesn't seem to be affected by that since pre-commit doesn't seem to isolate non-python programs.
There was a problem hiding this comment.
@Dr-Irv I think you set the pyright github action for https://github.com/pandas-dev/pandas-stubs up. How did you manage that pyright finds the virtual env? Running pyright directly (instead of using the github action) seemed to have no issue finding the virtual env.
I just copied the action from what Microsoft had set up in https://github.com/microsoft/python-type-stubs/blob/main/.github/workflows/test.yml . I think since it is a sequence of steps, the virtual env is automatically set up from the previous steps.
jreback
added
CI
|
Closing this. Will try to add mypy to the manual pre-commit stage and then run mypy&pyright through pre-commit. |
|
Thanks @twoertwein. No objection adding it via this PR if the pre-commit version doesn't work out |
This could slow down development. When I run |
|
This was implemented as a |
pyright has a github action integration https://github.com/microsoft/pyright/blob/main/docs/ci-integration.md
I added the
--skipunannotatedoption to prevent pyright from trying to infer missing return annotation (mypy doesn't do that either). That does not make a difference at the moment but it cuts reportGeneralTypeIssues errors in half (if it was enabled).