-
-
Notifications
You must be signed in to change notification settings - Fork 30k
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
bpo-32108: Don't clear configparser values if key is assigned to itself #7588
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.
Thanks for the updated PR and tests! I think we can do even better :-)
Lib/configparser.py
Outdated
self._defaults.clear() | ||
elif key in self._sections: | ||
elif key in self._sections and self._sections[key] != 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 will slow down the general case because !=
checks whether all keys and values in a mapping are the same.
The problem outlined in BPO-32108 only happens if value
is a SectionProxy. Only in this case clearing the underlying dictionary will cause the subsequent read_dict()
to fail.
So, instead of those checks, on line 967 make the following check:
if key in self and self[key] is value:
return
self[key]
returns a SectionProxy for the given section. If the value
is that same proxy, we don't need to do anything.
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.
@ambv Thanks for the suggestion and the explanation!
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
I have made the requested changes; please review again. |
Thanks for making the requested changes! @ambv: please review the changes made to this pull request. |
Thanks! ✨ 🍰 ✨ |
https://bugs.python.org/issue32108