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

Drop pyright type checking #381

Closed
duckontheweb opened this issue Jun 1, 2021 · 2 comments · Fixed by #417
Closed

Drop pyright type checking #381

duckontheweb opened this issue Jun 1, 2021 · 2 comments · Fixed by #417
Assignees
Labels
discussion An issue to capture a discussion

Comments

@duckontheweb
Copy link
Contributor

Starting with c073712, we started using pyright instead of mypy for type checking in our tests/CI. There were some good reasons for this (outlined in the comments of that commit), but since then we have re-adopted mypy (via #337).

This issue exists to discuss whether we should drop the use of pyright for type checking in our tests and CI.

I'm personally +1 for dropping pyright for the following reasons:

  • Using pyright adds a Node.js/npm dependency to a project that is otherwise purely Python
  • mypy seems to be the stricter of the two type checkers, so it's not obvious to me that pyright is adding anything
@duckontheweb duckontheweb added the discussion An issue to capture a discussion label Jun 1, 2021
@schwehr
Copy link
Collaborator

schwehr commented Jun 1, 2021

I'm +1 on dropping pyright just because of the Node/npm dependency.

I created #382 for the related (but definitely distinct) issue of pytype versus mypy.

Not to mention that life will be better when the minimum python version is 3.10 (sigh...) when the typing system just understand how to handle two types that refer to each other.

@l0b0
Copy link
Contributor

l0b0 commented Jun 3, 2021

I'm generally in favour as well, but I should probably mention that the pre-commit PR includes a commit which adds package.json, package-lock.json and instructions for installing the Node modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion An issue to capture a discussion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants