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

Fix typing error #1359

Merged
merged 1 commit into from
Nov 16, 2023
Merged

Fix typing error #1359

merged 1 commit into from
Nov 16, 2023

Conversation

jwodder
Copy link
Member

@jwodder jwodder commented Nov 16, 2023

I suspect the error in question only started being emitted when mypy 1.7.0 was released on the 10th.

@jwodder jwodder added the tests Add or improve existing tests label Nov 16, 2023
Copy link

codecov bot commented Nov 16, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cb86c9c) 89.14% compared to head (2bb9c52) 89.13%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1359      +/-   ##
==========================================
- Coverage   89.14%   89.13%   -0.02%     
==========================================
  Files          75       75              
  Lines       10266    10265       -1     
==========================================
- Hits         9152     9150       -2     
- Misses       1114     1115       +1     
Flag Coverage Δ
unittests 89.13% <ø> (-0.02%) ⬇️

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.

@@ -44,7 +44,7 @@
@devel_debug_option()
@map_to_click_exceptions
def move(
paths: tuple[str],
paths: tuple[str, ...],
Copy link
Member

Choose a reason for hiding this comment

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

for my own sake, so the old one is a tuple of a single str and new one is tuple of strings or tuple with the first element a str?

Copy link
Member Author

Choose a reason for hiding this comment

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

tuple[str] is a tuple of length 1 containing a str. tuple[str, ...] is a tuple of any length where all elements are strs.

Mypy was complaining because we we doing this inside the function:

if len(paths) < 2:
raise ValueError("At least two paths are required")

and len() of a one-element tuple is always less than 2, making the line after the raise unreachable.

Copy link
Member

Choose a reason for hiding this comment

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

so it is the

dandi/cli/cmd_move.py:90: error: Statement is unreachable  [unreachable]
        move_mod.move(
        ^
Found 1 error in 1 file (checked 77 source files)

one. got it. thank you for the explanation

@yarikoptic yarikoptic merged commit 29d2c9c into master Nov 16, 2023
24 of 25 checks passed
@yarikoptic yarikoptic deleted the type-fix branch November 16, 2023 20:12
Copy link

🚀 PR was released in 0.58.1 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released tests Add or improve existing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants