CI: run pyright
State: closed

Run pyright in strict mode with a very long list of rules that need to be ignored. The idea is to enable more rules over time.

pre-commit seamlessly installs pyright  (depends on node.js) but running pyright as part of the pre-commit slows the pre-commit down by ~85s.


Labels:
name: CI
description: Continuous Integration
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
Some points to consider:

- As far as I know, pyright does not have an option to ignore type errors in specific lines. If it is a false positive, pyright will most likely fix it within days(!). If it is a type error, pyright recommends to use `cast`.
- Pyright can exclude/include files/directories.
- @erictraut  [recommends](https://github.com/microsoft/pyright/issues/2315#issuecomment-924101740) to focus on enabling `reportGeneralTypeIssues` for at least a small list of files (and then growing the list of files from there).

While this PR enables very few rules for all files (except for the tests), it would make sense in subsequent PRs to prioritize fixing `reportGeneralTypeIssues`: do not cover all files with pyright but enable more rules (specifically `reportGeneralTypeIssues`).
You can ignore type errors on a specific line using a `# type: ignore` comment at the end of that line. This is documented in PEP 484 and is supported by all Python type checkers.
> You can ignore type errors on a specific line using a `# type: ignore` comment at the end of that line. This is documented in PEP 484 and is supported by all Python type checkers.

Thank you for the correction! I remembered one issue on pyright that mentioned that there wasn't this possibility.
I noticed that you added "pandas/tests" to the "ignore" section of the configuration. This tells pyright to analyze these files but suppress reporting any diagnostics in them. I recommend moving it to "exclude" instead, which tells pyright to avoid analyzing them unless they are explicitly imported by other modules that are included in the analysis set. Making this change drops the total analysis time from 67 seconds to 19 seconds on my machine. 

```
exclude = ["pandas/tests"]
```

I'll also point out that as more type annotations are added to the code base, the overall analysis time will decrease. Currently, pyright needs to perform a lot of expensive computations to infer function return types based on the function implementation and call-site argument types. I would anticipate that the total analysis time would drop by 50% or more as you add more type annotations.
> Making this change drops the total analysis time from 67 seconds to 19 seconds on my machine.

That's amazing, thank you for noticing that! 19s should be more than fast enough to run pyright with all pre-commit hooks.
mypy used to be in the pre-commit but was removed in #35066 (because of running in a different environment and bugs(?) in mypy https://github.com/pre-commit/pre-commit/issues/1580#issuecomment-679413936).

We probably need to add all the type-stub packages as dependencies to the pyright hook (pyright comes with its copy of typeshed, so we might not need to install various type-stub packages(?)). At a minimum, we probably need to add `numpy>1.21.0` to have access to `numpy.typing`.

edit: It seems that pre-commit doesn't isolate pyright from the current python environment because it is a node.js application(?)! That's very convenient, especially because it seems that pre-commit doesn't support installing node dependencies and python dependencies for the same check. 
thanks @twoertwein very nice. 
pls open issues as appropriate (or a single with check boxes) to enable features on pyright checking.