Do not use mutables as default parameters #21402
Conversation
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. 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.
|
Good catch! Some of that code is ancient, so we probably just haven't noticed. My only question (for the group at large) is where did |
|
By the way, do you happen to know of any style or lint checker that we could turn on to catch this in the future? |
|
It looks like I think the two files have diverged - at least the git logs are different. I seem to recall DeepSource.io does catch this issue. I have not fixed occurrences where the mutable is not modified by the function, because I don't want my changes to be too intrusive - not out of laziness 😄 |
|
This PR is affected by a re-writing of our history to remove a large number of accidentally committed files see discourse for details. To recover this PR it will need be rebased onto the new default branch (main). There are several ways to accomplish this, but we recommend (assuming that you call the matplotlib/matplotlib remote git remote update
git checkout main
git merge --ff-only upstream/main
git checkout YOUR_BRANCH
git rebase --onto=main upstream/old_master
# git rebase -i main # if you prefer
git push --force-with-lease # assuming you are tracking your branchIf you do not feel comfortable doing this or need any help please reach out to any of the Matplotlib developers. We can either help you with the process or do it for you. Thank you for your contributions to Matplotlib and sorry for the inconvenience. |
|
Should be rebased on |
|
Thank you for being the first-time contributor test-subject for these instructions @DimitriPapadopoulos 😃 . |
I have fixed two categories of code: * examples, * functions that modify or risk modifying a mutable default argument.
|
Thanks @DimitriPapadopoulos! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again. |
|
Yes, it's true the mutables are not modified in these occurrences. My concern was the examples, which should be fixed because users re-use them blindly. |
PR Summary
I have fixed two categories of code:
Articles that discuss why mutable default arguments should be avoided:
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).