🩹 Remove mutable default params #959
Conversation
Mutable defaults for function parameters is a bad practice that can lead to unexpected and tricky bugs. Defaults being evaluated only once at import time, a new list is created once when the function is defined and the same list is used in each successive call. Mutating the `tags` argument would result in mutating the default for subsequent calls of the method. See https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments
|
The documentation is not available anymore as the PR was closed or merged. |
src/huggingface_hub/hf_api.py
Outdated
| @@ -222,7 +222,7 @@ def __init__( | |||
| self.modelId = modelId | |||
| self.sha = sha | |||
| self.lastModified = lastModified | |||
| self.tags = tags | |||
| self.tags = tags if tags is not None else [] | |||
There was a problem hiding this comment.
| self.tags = tags if tags is not None else [] | |
| self.tags = tags |
I'd even do this personally (i.e. self.tags can be None)
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
SBrandeis
force-pushed
the
default-mutable-argument-bad
branch
from
a76fbc8 to
ef48539
Compare
|
After discussing with @julien-c, it happens that sometimes the API does not return |
Co-authored-by: Julien Chaumond <julien-c@users.no-reply.huggingface.co>
|
Good catch since this can really create unexpected behaviours. I would really suggest that we start tracking these errors automatically using flake8-bugbear. It is a plugin for flake8 that finds "bad patterns" in code. In particular
Any opinion about it ? If no-one sees a problem here I can create a PR to enable it. |
|
Personally, no objection to your proposal @Wauplin |
|
I'm in favor of @Wauplin's suggestion 👍 |
|
Done as part of #967. I'm closing it without merging. |
Using mutable defaults for function parameters is a bad practice that can lead to unexpected and tricky bugs.
Defaults are evaluated only once at import time. A new list is created once when the function is defined, and the same list is used in each successive call. Mutating the
tagsargument would result in mutating the default for subsequent calls of the method.See https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments