-
-
Notifications
You must be signed in to change notification settings - Fork 547
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
chore(mypy): Fix type annotation #751
Conversation
e41a438
to
f0ff64a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #751 +/- ##
=======================================
Coverage 96.38% 96.38%
=======================================
Files 10 10
Lines 249 249
Branches 7 7
=======================================
Hits 240 240
Misses 9 9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -49,7 +49,7 @@ def invoke_cli_app(parsed_cli_args: Namespace) -> ReturnCodeType: | |||
category=UserWarning, | |||
) | |||
|
|||
dirs = [] | |||
dirs: list[str] = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too pythonish for me 🤣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<variable_name>[: <variable_type>] = <variable_value>
Actually, I think that = <variable_value>
is optional too, but I can't remember when I saw such usage last time. It could be forbidden by most basic linters, tbh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not forbidden. You can declare a type of a variable before you actually initialize it. For most simple types, the type checkers like MyPy would infer the type of the data intended to be stored there. But when it's an empty container, like a list, it's ambiguous. dirs = []
made MyPy think the type was list[typing.Any]
while a more precise type should be list[str]
.
When declaring typing, the guidance is to use a narrow type when returning things from functions but wide types for the incoming function arguments. It allows passing more shapes of data into functions while being accurate regarding what they return. Individual variables should also be as accurate as possible.
This PR is included in version 1.97.0 🎉 |
Description of your changes
MyPy found that
dirs
is not annotated. This PR fix it.How can we test changes
coverage improved