Fix mutable default arguments in backend_svg.py#31141
Fix mutable default arguments in backend_svg.py#31141anntzer merged 1 commit intomatplotlib:mainfrom
Conversation
|
Thank you for opening your first PR into Matplotlib! If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. You can also join us on gitter for real-time discussion. For details on testing, writing docs, and our review process, please see the developer guide. We strive to be a welcoming and open project. Please follow our Code of Conduct. |
|
I noticed the checks failed, but they appear to be unrelated to my changes in backend_svg.py. |
|
@tacaswell Thanks for the approval! I see that @anntzer opened #31143, which refactors this class and effectively resolves the same issue. Do you prefer to merge this PR as a targeted fix first, or should I close this in favor of the refactor? Happy to go with whatever works best for the codebase. |
anntzer
left a comment
anntzer
left a comment
There was a problem hiding this comment.
I can rebase my PR on top of this one, no problem. Can you squash your two commits together, though?
llukito
force-pushed
the
fix-svg-mutable-defaults
branch
from
fefab51 to
fded0d5
Compare
|
Squashed and force-pushed, thanks for the review. |
|
test failures are spurious. |
4 participants
Description
This PR fixes a mutable default argument issue in
lib/matplotlib/backends/backend_svg.py.In the
XMLWriterclass, thestartandelementmethods were using a mutable dictionary (attrib={}) as a default argument.While the current implementation may not be strictly mutating this dictionary in place, using a mutable default is an anti-pattern that creates a shared state across function calls. This change replaces it with
Noneto ensure a fresh dictionary is created for every call, preventing potential side effects in future code changes.Changes
def start(..., attrib={}, ...)todef start(..., attrib=None, ...)def element(..., attrib={}, ...)todef element(..., attrib=None, ...)if attrib is None: attrib = {}to initialize the dictionary safely inside the methods.