-
Notifications
You must be signed in to change notification settings - Fork 228
Add typing to aiida.cmdline.params module #6952
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
Conversation
@@ -13,7 +13,7 @@ | |||
import click | |||
|
|||
|
|||
def edit_multiline_template(template_name, comment_marker='#=', extension=None, **kwargs): |
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.
This was an actual bug, extension cannot be None
. It was not a bug in practice since this function is always being called with a extension
parameter explicitly specified.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6952 +/- ##
==========================================
+ Coverage 79.16% 79.18% +0.03%
==========================================
Files 566 566
Lines 43461 43476 +15
==========================================
+ Hits 34400 34422 +22
+ Misses 9061 9054 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Thanks a lot @danielhollas
I left a few comments, just to understand.
if self.name is None: | ||
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.
This doesn't seem to be related to typing, right? perhaps belongs to #6883 ?
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.
Without this early return I get this error:
src/aiida/cmdline/params/options/interactive.py:110: error:
Argument 1 to "get_parameter_source" of "Context" has incompatible type "str | None"; expected "str" [arg-type]
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 see, alright let's keep it then 👍
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.
Thanks a lot @danielhollas
All good 👍
This will hopefully make it a little bit safer to depend click internals. I needed to add a bunch to
type: ignore
statements, but that's a better trafeoff than not checking these files at all.Split from #6883