bpo-32108: Don't clear configparser values if key is assigned to itself#7588
Conversation
ambv
left a comment
ambv
left a comment
There was a problem hiding this comment.
Thanks for the updated PR and tests! I think we can do even better :-)
| if key == self.default_section and self._defaults != value: | ||
| 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.
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.
@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! ✨ 🍰 ✨ |
4 participants
https://bugs.python.org/issue32108