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
DM-37025: Enable type checking for click commands #755
Conversation
) -> ContextManager[ProgressBar[_T]]: | ||
# Docstring inherited. | ||
return click.progressbar(iterable=iterable, length=total, label=desc, **self._kwargs) | ||
return click.progressbar(iterable=iterable, length=total, label=desc, **self._kwargs) # type: ignore |
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.
I wasn't really sure what to do with this one. The error was:
Incompatible return value type (got "Union[click._termui_impl.ProgressBar[_T], click._termui_impl.ProgressBar[int]]", expected "AbstractContextManager[lsst.daf.butler.core.progress.ProgressBar[_T]]") [return-value]
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.
I'm not sure whether it complains about click.ProgressBar
not matching daf.butler.ProgressBar
protocol, or that click.ProgressBar
does not inherit from ContextManager
.
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.
The docs for click.progressbar
say it returns ProgressBar[V]
and then says "This function creates an iterable context manager", so for all I know the type annotation is wrong in click
.
85eb354
to
d1a34c2
Compare
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.
Looks good, couple of minor comments.
) -> ContextManager[ProgressBar[_T]]: | ||
# Docstring inherited. | ||
return click.progressbar(iterable=iterable, length=total, label=desc, **self._kwargs) | ||
return click.progressbar(iterable=iterable, length=total, label=desc, **self._kwargs) # type: ignore |
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.
I'm not sure whether it complains about click.ProgressBar
not matching daf.butler.ProgressBar
protocol, or that click.ProgressBar
does not inherit from ContextManager
.
d1a34c2
to
9fabea2
Compare
Codecov ReportBase: 85.30% // Head: 85.29% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #755 +/- ##
==========================================
- Coverage 85.30% 85.29% -0.01%
==========================================
Files 260 260
Lines 34424 34468 +44
Branches 5802 5810 +8
==========================================
+ Hits 29366 29401 +35
- Misses 3819 3824 +5
- Partials 1239 1243 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Checklist
doc/changes