Set the defaults argument to None to fix dangerous-default-value linter warning #286
Conversation
There was a problem hiding this comment.
The default value being missing has been deprecated since 2020
I think a better solution for this would be to remove the default for the default value. The only place we call it is guaranteed to fill the value.
There's one place with a known usage of this the Volume that I'll submit a PR to resolve.
We could keep the type hint for it though.
| @@ -117,7 +117,9 @@ def check_args_for_activation(cls, cli_args): | |||
| return True if cli_args.get(cls.get_name()) else False | |||
|
|
|||
| @staticmethod | |||
| def register_arguments(parser, defaults={}): | |||
| def register_arguments(parser, defaults:dict=None): | |||
| if defaults is None: | |||
There was a problem hiding this comment.
When the user overrides this function this won't be called.
| @@ -117,7 +117,9 @@ def check_args_for_activation(cls, cli_args): | |||
| return True if cli_args.get(cls.get_name()) else False | |||
|
|
|||
| @staticmethod | |||
| def register_arguments(parser, defaults={}): | |||
| def register_arguments(parser, defaults:dict=None): | |||
There was a problem hiding this comment.
| def register_arguments(parser, defaults:dict=None): | |
| def register_arguments(parser, defaults: dict=None): |
Need a space for readability.
This is a dangerous practice becuse the default values could be mutated. And it should always be passed in. Supercedes #286
This is a dangerous practice becuse the default values could be mutated. And it should always be passed in. Supercedes #286
This is a dangerous practice becuse the default values could be mutated. And it should always be passed in. Supercedes #286
|
With #288 merged this is no longer necessary and should remove your linting issues. It looks like you're adding type hints too. If you would like to take the time to extend the base class along those lines that would be appreciated in a follow up PR. Thanks for flagging this and raising it here. |
I made a rocker extension the other day and was getting the dangerous-default-value linter warning on the register_arguments() function.
https://pylint.pycqa.org/en/latest/user_guide/messages/warning/dangerous-default-value.html
I applied the standard fix it my code to remove the warning, but thought it could be fixed upstream as well so when people make a new extension they will not have to fix it themselves.