-
Notifications
You must be signed in to change notification settings - Fork 202
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
make EasyConfigParser.get_config_dict return a copy rather than a reference #3692
Conversation
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 would have done it at a higher level (i.e. in the parser which has-a format object to avoid duplicating this, but this works too. Thanks!
For future reference as written in Slack:
The faster solution would be to copy the dict only on (potential) modification, e.g. when it is assigned into the EC (set_keys
) because now we copy even if we only read from it (e.g. in the EC test suite). However it was decided to go for the safer solution and avoid the potential for accidental modification at the cost of a (comparatively cheap) copy of a dict
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.
lgtm
Going in, thanks @boegel! |
fixes #3691