Configure Pyright #1246
Merged
Configure Pyright #1246
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
victorlin
force-pushed
the
victorlin/add-type-checking
branch
2 times, most recently
from
8c19b75
to
2a74460
Compare
victorlin
force-pushed
the
victorlin/use-pyright
branch
2 times, most recently
from
67b16d2
to
3469831
Compare
|
(1 year later - I must have missed this at the time) I think this is a great direction to go in. My not-extensive understanding of type checking in python indicates pyright is preferred over mypy. It seems strange to me to use both typecheckers in the same project, but this seems to be rather common in python. |
victorlin
force-pushed
the
victorlin/use-pyright
branch
from
3469831
to
fda57c5
Compare
victorlin
commented
May 17, 2024
There was a problem hiding this comment.
I've changed the direction of this PR from trying to address all violations to just setting up Pyright and ignoring violations explicitly. This reflects similar work done in nextstrain/nextstrain.org#778.
Here's commits from the old direction: 9e8c9e0...3469831 and new direction: fda57c5
victorlin
force-pushed
the
victorlin/use-pyright
branch
from
2205fe0
to
062fb9b
Compare
genehack
reviewed
May 17, 2024
Add the necessary configuration including rule exceptions to make the
linter run without error on the code as-is. This means the check is
mostly useless except to prevent any violations of rules that aren't
currently violated, but it allows for incremental adoption of rules
rather than needing to address all violations at once.
List of exceptions determined using command:
npx pyright --outputjson \
| jq --raw-output '.generalDiagnostics.[] | .rule' \
| sort -u
Invocation via pytest mirrors that of mypy. Both were ported from
existing setup in Nextstrain CLI.¹
¹ <https://github.com/nextstrain/cli/blob/14a392eaf40f779a7ef9431bbe0a116e93d98580/doc/development.md?plain=1#L143>
I'm unsure why these don't show up locally on the same Pyright version. They were flagged by CI so I'm adding them to the list from the previous commit.
victorlin
force-pushed
the
victorlin/use-pyright
branch
from
062fb9b
to
e5ced1c
Compare
Closed
17 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of proposed changes
Add the necessary configuration including rule exceptions to make the linter run without error on the code as-is. This means the check is mostly useless except to prevent any violations of rules that aren't currently violated, but it allows for incremental adoption of rules rather than needing to address all violations at once.
List of exceptions determined using command:
Pyright supports some checks and type inference that Mypy does not¹.
¹ https://github.com/microsoft/pyright/blob/1.1.316/docs/mypy-comparison.md
Checklist
Add a message in CHANGES.md summarizing the changes in this PR that are end user focused. Keep headers and formatting consistent with the rest of the file.no functional changes