-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
ens.utils.dict_copy
only operates on keyword arguments
#1423
Comments
Can I work on it? @pipermerriam? |
I'd like to take this if it is still open! |
So I was thinking about taking this one on but then started thinking. Wouldn't this be a breaking change as consumers of the library could be relying on the @dict_copy to not mutate the dictionary that they are passing in to things like main.setup_address. I can make the change but just wanted to make sure I made it on the right branch as it might not be appropriate to make the change on master. |
Fixed in PR #1998 |
What was wrong?
There is a
dict_copy
decorator used in theens
module. It is intended to allow us to write methods like this:Normally using a mutable value like a dictionary as a default argument is problematic because the same mutable object is used across each call. This decorator institutes a
copy
of this, but only if it is passed as a keyword argument.There are at least some places where
dict_copy
is used but where the parameter that needs to be copied is not enforced to be a keyword argument, meaning that any calls that use it positionally will not benefit from the protection.How can it be fixed?
I'm inclined to suggest removing the decorator entirely and changing all places it was used to follow the standard pattern of:
The text was updated successfully, but these errors were encountered: