Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch from mypy to pyright for static type checking #2451

Closed
6 tasks
namurphy opened this issue Jan 8, 2024 · 3 comments
Closed
6 tasks

Switch from mypy to pyright for static type checking #2451

namurphy opened this issue Jan 8, 2024 · 3 comments
Labels
CI Related to continuous integration proposal Proposals that need to be decided upon before implementation refactoring ♻️ Improving an implementation without adding new functionality static type checking

Comments

@namurphy
Copy link
Member

namurphy commented Jan 8, 2024

Description of improvement

We recently added mypy as a static type checker in our suite of continuous integration tests. This issue proposes to switch to pyright for static type checking instead.

Motivation

Before adding mypy to the CI suite, I didn't really look into alternatives to mypy. From the information discussions I've seen online, generally there seems to be a preference for pyright over mypy with respect to quality of error messages, performance, quality of the code base, and responsiveness of the maintainers to bug reports. The quality of error messages is particularly important because we have a lot of new contributors.

Implementation strategy

The documentation for pyright discusses getting started.

Additional context

Original issue for mypy: #268.

@namurphy namurphy added refactoring ♻️ Improving an implementation without adding new functionality status: on hold Issues & PRs that are being intentionally delayed CI Related to continuous integration static type checking labels Jan 8, 2024
@namurphy
Copy link
Member Author

namurphy commented Jan 8, 2024

I'm labeling this as "on hold" for the moment. I'm thinking about doing a prototype PR.

@namurphy namurphy added the proposal Proposals that need to be decided upon before implementation label Jan 8, 2024
@namurphy
Copy link
Member Author

namurphy commented Jan 8, 2024

Some initial impressions after trying out pyright:

  • I had difficulty installing pyright. It ended up working when I did conda install pyright.
  • I find the mypy documentation to be easier to glean information from.
  • mypy has a customization option that will print out the code and show the location of the error, which I found quite helpful. I didn't see an option for pyright.
  • pyright doesn't have a plugin system, though I don't think that would affect us too much.
  • pyright seems pretty thorough. It found 1306 errors in "basic" mode, excluding tests directorires.
  • There should probably be only one static type checker in our CI since the type: ignore comments have different error codes. However, if we're going through the process of adding type hint annotations in a file, it might be worth both running pyright and mypy. So, maybe it's worth adding a tox environment for pyright too?
  • Enabling a static type checker on a large existing code base is going to be a pain regardless of which one you choose. 🙃

I'm not convinced yet that it would be worth going through the effort to switch.

@namurphy
Copy link
Member Author

Closing this since I don't think switching to pyright will be worth the effort, at least for the time being.

@namurphy namurphy reopened this Jan 11, 2024
@namurphy namurphy closed this as not planned Won't fix, can't repro, duplicate, stale Jan 11, 2024
@namurphy namurphy removed the status: on hold Issues & PRs that are being intentionally delayed label Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Related to continuous integration proposal Proposals that need to be decided upon before implementation refactoring ♻️ Improving an implementation without adding new functionality static type checking
Projects
None yet
Development

No branches or pull requests

1 participant