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

chore(mypy): Fix type annotation #751

Merged
merged 1 commit into from
Jan 9, 2025
Merged

chore(mypy): Fix type annotation #751

merged 1 commit into from
Jan 9, 2025

Conversation

MaxymVlasov
Copy link
Collaborator

Description of your changes

MyPy found that dirs is not annotated. This PR fix it.

How can we test changes

coverage improved

@MaxymVlasov MaxymVlasov requested a review from yermulnik as a code owner January 9, 2025 23:00
Copy link

codecov bot commented Jan 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.38%. Comparing base (1c9823e) to head (f0ff64a).
Report is 1 commits behind head on master.

✅ 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           
Flag Coverage Δ
CI-GHA 96.38% <100.00%> (ø)
MyPy 91.05% <100.00%> (+0.40%) ⬆️
OS-Linux 96.38% <100.00%> (ø)
OS-Windows 100.00% <100.00%> (ø)
OS-macOS 100.00% <100.00%> (ø)
Py-3.10.11 100.00% <100.00%> (ø)
Py-3.10.16 100.00% <100.00%> (ø)
Py-3.11.11 100.00% <100.00%> (ø)
Py-3.11.9 100.00% <100.00%> (ø)
Py-3.12.8 100.00% <100.00%> (ø)
Py-3.13.1 96.38% <100.00%> (ø)
Py-3.9.13 100.00% <100.00%> (ø)
Py-3.9.21 100.00% <100.00%> (ø)
VM-macos-13 100.00% <100.00%> (ø)
VM-macos-14 100.00% <100.00%> (ø)
VM-ubuntu-24.04 100.00% <100.00%> (ø)
VM-ubuntu-latest 91.05% <100.00%> (+0.40%) ⬆️
VM-windows-2025 100.00% <100.00%> (ø)
pytest 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -49,7 +49,7 @@ def invoke_cli_app(parsed_cli_args: Namespace) -> ReturnCodeType:
category=UserWarning,
)

dirs = []
dirs: list[str] = []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too pythonish for me 🤣

Copy link
Collaborator Author

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.

Copy link
Contributor

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.

@MaxymVlasov MaxymVlasov merged commit 5181957 into master Jan 9, 2025
63 checks passed
@MaxymVlasov MaxymVlasov deleted the fix/mypy_in_hook branch January 9, 2025 23:59
@antonbabenko
Copy link
Owner

This PR is included in version 1.97.0 🎉

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

Successfully merging this pull request may close these issues.

4 participants