Addressing issue #7434: Adding kwarg to Figure.clf() to refetch rcParams and update existing object #21097
Conversation
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Fixes matplotlib#20516 PGF's `random steps` decoration seems to be the most similar, but does not exactly match the behaviour described in matplotlib's docs. Therefore I repurposed the `randomness` argument as a seed to give control on how the line looks afterwards.
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Switch documented deprecations in mathtext by `__getattr__` deprecations
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
…_output API: rename draw_no_output to draw_without_rendering
Make warning for no-handles legend more explicit.
Remove unused HostAxes._get_legend_handles.
Redirect to new 3rd party packages page
legend_handler_map cleanups.
Clean up some Event class docs.
- parse() already normalizes prop=None to prop=FontProperties(). - We don't need to explicitly attach a canvas to a plain Figure() before calling savefig() anymore; FigureCanvasBase is always attached and handles the dispatching to the correct concrete canvas class.
According to the docs, this function was added in GTK 2.2, so probably was only needed when we supported GTK2. It also no longer exists in GTK4.
Since these examples don't need to support the very oldest GTK3, I took the opportunity to rewrite them using the recommended Application-styled model.
The `Gtk.Window.destroy` doesn't work, but `.close` correctly triggers our cleanup.
Use sysconfig directly instead of through distutils
Support marker="none" to mean "no marker".
FIX: Break links between twinned axes when removing
Explicit registration of canvas-specific tool subclasses.
This seems to be failing a bit more often today, so hopefully this will help a little.
Cache build dependencies on Circle
Fix bug in shape assignment
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.
| @@ -119,7 +120,63 @@ def test_clf_keyword(): | |||
|
|
|||
| fig2, ax2 = plt.subplots(2, 1, num=1, clear=True) | |||
| assert fig0 is fig2 | |||
| assert [t.get_text() for t in fig2.texts] == [] | |||
There was a problem hiding this comment.
I think you dropped this line by accident?
| @@ -3,7 +3,7 @@ | |||
| from pathlib import Path | |||
| import platform | |||
| from threading import Timer | |||
| from types import SimpleNamespace | |||
| from types import BuiltinFunctionType, SimpleNamespace | |||
There was a problem hiding this comment.
I don't see where you are using this:
| from types import BuiltinFunctionType, SimpleNamespace | |
| from types import SimpleNamespace |
| raise ValueError( | ||
| f"figure size must be positive finite not {figsize}" | ||
| ) | ||
| self.bbox_inches = Bbox.from_bounds(0, 0, *figsize) |
There was a problem hiding this comment.
OK, this is all copied from __init__ which is great, except its usually better not to copy (suppose we want to add a new rcParam, now we need to add it in two places). Can this be cleanly refactored into a helper that is called by both __init__ and clf? Thanks!
|
Well, this is what I suggested in #7434! If we are going to do this, I still think this is the way to do it, but do we think this is a good idea? @nasgold In your comment you seem rather down on this feature! Given that this has been sitting from ~2016 with no additional agitation, maybe this is something that we do not actually need? This PR raises a question of if I agree with all of @jklymak 's line comments I also suspect you will need to run https://github.com/matplotlib/matplotlib/blob/master/tools/boilerplate.py to update the signature and docstring of the auto-generated pyplot functions. No matter where this lands, thank you for you work on this @nasgold ! Even if we ultimately close this PR (and the other draft PR to address #7434) and #7434 as "wont-fix", re-surfacing this for discussion is valuable. |
|
poppin to draft until the review comments are looked at. Feel free to pop back on the queue! |
|
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. |
|
I am going to close this PR because it has been quiet for more than a year, still needs the rebase from our history-rewrite issue, and because we still have not decided what "should" happen in these cases. Thank you for your work on this @nasgold even if we did not ultimately merge it. I hope we will hear from you again. |
PR Summary
Adding
refetch_rcparamskwarg toFigure.clf()that defaults toFalseto maintain existing, desired behavior. If set toTrue, however, theFigure.clf()call will re-consultrcParamsand update the already extant object. Some (myself included) might find the default behavior counterintuitive, and adding in this kwarg makes behavior either way more explicit.If the PR is accepted I will document the API change and make an entry in the "next_whats_new" file.
closes #7434 [@tacaswell edited to add GH xrefs]
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).