Repository: matplotlib/matplotlib
Pull request: Fix behaviour of Figure.clear() for SubplotParams
<!--
Thank you so much for your PR!  To help us review your contribution, please check
out the development guide https://matplotlib.org/devdocs/devel/index.html
-->

## PR summary

* Update `Figure.clear()` to reset the properties of `Figure.subplotpars`. 
* Add representation method for `matplotlib.gridspec.SubplotParams`
* Add `rc_default` parameter to the `SubplotParams.update`

Fixes #11059. This PR is a continuation of #11086 by @fredrik-1 

<!-- Please provide at least 1-2 sentences describing the pull request in detail
(Why is this change required?  What problem does it solve?) and link to relevant
issues and PRs.
Also please summarize the changes in the title, for example "Raise ValueError on
non-numeric input to set_xlim" and avoid non-descriptive titles such as "Addresses
issue #8576".
-->
## PR checklist
<!-- Please mark any checkboxes that do not apply to this PR as [N/A].-->

- [x] "closes #0000" is in the body of the PR description to [link the related issue](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
- [x] new and changed code is [tested](https://matplotlib.org/devdocs/devel/testing.html)
- [N/A] *Plotting related* features are demonstrated in an [example](https://matplotlib.org/devdocs/devel/document.html#write-examples-and-tutorials)
- [x] *New Features* and *API Changes* are noted with a [directive and release note](https://matplotlib.org/devdocs/devel/coding_guide.html#new-features-and-api-changes)
- [x] Documentation complies with [general](https://matplotlib.org/devdocs/devel/document.html#write-rest-pages) and [docstring](https://matplotlib.org/devdocs/devel/document.html#write-docstrings) guidelines

<!--We understand that PRs can sometimes be overwhelming, especially as the
reviews start coming in.  Please let us know if the reviews are unclear or
the recommended next step seems overly demanding, if you would like help in
addressing a reviewer's comments, or if you have been waiting too long to hear
back on your PR.-->


Problem statement:
figure.clf() and subplots_adjust
This might not be a bug but I doubt that it is an intended behavior.  `figure.clf() ` don't change the subplot parameters to any default values so it is not enough to do  `figure.clf() ` when testing to plot with or without for example  `tight_layout `. This is easy to fix by adding for example:
```
keys = ('left', 'right', 'bottom','top','wspace','hspace')
subplots_default = {key : rcParams['figure.subplot.' + key] for key in keys}
self.subplotpars.update(**subplots_default)
```
to figure.clf()

When I looked into this I also found that the docstrings for  `subplots_adjust ` and  `SubplotParams ` are kind of confusing and not really correct. The default is, except in  `SuplotParams.__ini__ ` (that most people probably newer use directly), not the rc parameters that it seems but the actual attributes in the  `SuplotParams ` object. So it is not trivial to start all over with the default rc variables after a change in the subplotparameters by for example tight_layout. 

- Would it be good to add an option in subplot_adjust to adjust to the default parameters? 
- Would it be good to add a get function for the subplotparameters? 

The documentation should probably be changed anyway. Are there any method to avoid writing almost the same things several times?  `figure.subplots_adjust ` and `plt.subplots_adjust` should have more or less the exact same docstring and the  `SuplotParams ` should also have a similar one.

Hints from issue discussion:
I'm not sure if this is intended behaviour or not.  If  understand what you are describing, it seems like `clf` should indeed reset those values.  Not sure about the utility of getters and setters etc.  

There is a tension in the docs between reproducibility and simplicity.  See the `legend` doc (in Master) for one way to make the docstrings consistent.  
A crazy way to reset the subplotpars to the rcParams' values would be
```
fig.subplotpars = type(fig.subplotpars)()
fig.subplots_adjust()
```
...probably not the recommended way. 
Similarly you may get all subplot parameters by just using the `SuplotParams`' `__dict__`

    print(fig.subplotpars.__dict__)

So given that those are really suboptimal "solutions", I think it would be good to 

* introduce a `reset_to_rc` function into `SubplotParams`. 
* let `clf` call this function.
* introduce either a figure method `fig.get_subplotparams()` or a subplot param method `fig.subplotparams.get_params()` returning a dictionary of the 6 subplotparams.
I added two labels because
- Documentation needs improvement in any case.
- We *might* want to modify the API.

This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!
The first and most important thing to do here is define what "clear" means for a figure. To do that, we should identify the use case ("Why clear and reuse a figure instead of just creating a new one?"). I see the main application when the figure is embedded in an application and you want to update the plot. Therefore:

`clear()` should (and currently does) remove:
- all content (subfigs, Axes, Legends, suptitle etc.)

`clear()` should (and currently does) keep:
- all "wiring", canvas, toolbar, embedding in parent figs (for subfigs), etc.
- figure size, dpi

I'm undecided on (and believe they are currently not reset):
- patch properties (facecolor, frame, ...)
- layout engine
- subplotpars

Whether they should be reset or not depends on how similar / different the new plot is. One can argue that these are similar fundamental visual properties like figure size and thus should not be reset. OTOH if the new should look substantially different, it's cumbersome to reset these manually.

Commit message: Update lib/matplotlib/tests/test_gridspec.py

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>

Patch to review:
diff --git a/doc/users/next_whats_new/subplots_adjust.rst b/doc/users/next_whats_new/subplots_adjust.rst
new file mode 100644
index 000000000000..94399f869aab
--- /dev/null
+++ b/doc/users/next_whats_new/subplots_adjust.rst
@@ -0,0 +1,9 @@
+Resetting the subplot parameters for figure.clear()
+---------------------------------------------------
+
+When calling `.Figure.clear()` the settings for `.gridspec.SubplotParams` are restored to the default values.
+
+The `.gridspec.SubplotParams` object has a new method to get the subplot parameters :meth:`~.SubplotParams.to_dict` and a
+method to reset the parameters to the defaults :meth:`~.SubplotParams.reset`
+
+(contributed by @eendebakpt based on work by @fredrik-1)
diff --git a/lib/matplotlib/figure.py b/lib/matplotlib/figure.py
index ec11e379db60..bf4e2253324f 100644
--- a/lib/matplotlib/figure.py
+++ b/lib/matplotlib/figure.py
@@ -989,6 +989,7 @@ def clear(self, keep_observers=False):
         self.texts = []
         self.images = []
         self.legends = []
+        self.subplotpars.reset()
         if not keep_observers:
             self._axobservers = cbook.CallbackRegistry()
         self._suptitle = None
diff --git a/lib/matplotlib/gridspec.py b/lib/matplotlib/gridspec.py
index 06f0b2f7f781..b753301d9280 100644
--- a/lib/matplotlib/gridspec.py
+++ b/lib/matplotlib/gridspec.py
@@ -740,22 +740,22 @@ def __init__(self, left=None, bottom=None, right=None, top=None,
 
         Parameters
         ----------
-        left : float
+        left : float, optional
             The position of the left edge of the subplots,
             as a fraction of the figure width.
-        right : float
+        right : float, optional
             The position of the right edge of the subplots,
             as a fraction of the figure width.
-        bottom : float
+        bottom : float, optional
             The position of the bottom edge of the subplots,
             as a fraction of the figure height.
-        top : float
+        top : float, optional
             The position of the top edge of the subplots,
             as a fraction of the figure height.
-        wspace : float
+        wspace : float, optional
             The width of the padding between subplots,
             as a fraction of the average Axes width.
-        hspace : float
+        hspace : float, optional
             The height of the padding between subplots,
             as a fraction of the average Axes height.
         """
@@ -786,3 +786,12 @@ def update(self, left=None, bottom=None, right=None, top=None,
             self.wspace = wspace
         if hspace is not None:
             self.hspace = hspace
+
+    def reset(self):
+        """Restore the positioning parameters to the default values"""
+        for key in self.to_dict():
+            setattr(self, key, mpl.rcParams[f'figure.subplot.{key}'])
+
+    def to_dict(self):
+        """Return the subplot parameters as a dict."""
+        return self.__dict__.copy()
diff --git a/lib/matplotlib/gridspec.pyi b/lib/matplotlib/gridspec.pyi
index 3bf13ee17c4e..04de229f4d42 100644
--- a/lib/matplotlib/gridspec.pyi
+++ b/lib/matplotlib/gridspec.pyi
@@ -158,3 +158,7 @@ class SubplotParams:
         wspace: float | None = ...,
         hspace: float | None = ...,
     ) -> None: ...
+    def to_dict(
+        self,
+    ) -> dict[str, float]: ...
+    def reset(self) -> None: ...
diff --git a/lib/matplotlib/tests/test_figure.py b/lib/matplotlib/tests/test_figure.py
index 014eb2cf23d0..496ac0a071ec 100644
--- a/lib/matplotlib/tests/test_figure.py
+++ b/lib/matplotlib/tests/test_figure.py
@@ -1770,6 +1770,24 @@ def test_warn_colorbar_mismatch():
         subfig3_1.colorbar(im4_1)
 
 
+def test_clf_subplotpars():
+    keys = ('left', 'right', 'bottom', 'top', 'wspace', 'hspace')
+    rc_params = {key: plt.rcParams['figure.subplot.' + key] for key in keys}
+
+    fig = plt.figure(1)
+    fig.subplots_adjust(**{k: v+0.01 for k, v in rc_params.items()})
+    fig.clf()
+    assert fig.subplotpars.to_dict() == rc_params
+
+
+def test_suplots_adjust_incremental():
+    fig = plt.figure()
+    fig.subplots_adjust(left=0)
+    fig.subplots_adjust(right=1)
+    assert fig.subplotpars.left == 0
+    assert fig.subplotpars.right == 1
+
+
 def test_set_figure():
     fig = plt.figure()
     sfig1 = fig.subfigures()
diff --git a/lib/matplotlib/tests/test_gridspec.py b/lib/matplotlib/tests/test_gridspec.py
index deda73c3b6ab..625a79816ed3 100644
--- a/lib/matplotlib/tests/test_gridspec.py
+++ b/lib/matplotlib/tests/test_gridspec.py
@@ -1,3 +1,4 @@
+import matplotlib
 import matplotlib.gridspec as gridspec
 import matplotlib.pyplot as plt
 import pytest
@@ -9,6 +10,13 @@ def test_equal():
     assert gs[:, 0] == gs[:, 0]
 
 
+def test_update():
+    gs = gridspec.GridSpec(2, 1)
+
+    gs.update(left=.1)
+    assert gs.left == .1
+
+
 def test_width_ratios():
     """
     Addresses issue #5835.
@@ -27,6 +35,23 @@ def test_height_ratios():
         gridspec.GridSpec(1, 1, height_ratios=[2, 1, 3])
 
 
+def test_SubplotParams():
+    s = gridspec.SubplotParams(.1, .1, .9, .9)
+    assert s.left == 0.1
+
+    s.reset()
+    assert s.left == matplotlib.rcParams['figure.subplot.left']
+
+    with pytest.raises(ValueError, match='left cannot be >= right'):
+        s.update(left=s.right + .01)
+
+    with pytest.raises(ValueError, match='bottom cannot be >= top'):
+        s.update(bottom=s.top + .01)
+
+    with pytest.raises(ValueError, match='left cannot be >= right'):
+        gridspec.SubplotParams(.1, .1, .09, .9)
+
+
 def test_repr():
     ss = gridspec.GridSpec(3, 3)[2, 1:3]
     assert repr(ss) == "GridSpec(3, 3)[2:3, 1:3]"


Code review comments:
diff_hunk: @@ -0,0 +1,9 @@
+Resetting the subplot parameters for figure.clear()
+---------------------------------------------------
+
+When calling `.Figure.clear()` the settings for `.gridspec.SubplotParams` are restored to the default values.
+
+The `.gridspec.SubplotParams` object has a new method to get the subplot parameters :meth:`~.SubplotParams.to_dict` and a
+method to reset the parameters to the defaults :meth:`~.SubplotParams.reset`
+
+(contributed by @eendebakpt based on work by @fredrik-1)
path: doc/users/next_whats_new/subplots_adjust.rst
text: @user1:
We don't have "(contributed by..)" in the what's new

```suggestion
```

@user1:
If you want to acknowledge someone, add their GitHub email to the commit message (which I admit is hard to find sometimes).

diff_hunk: @@ -786,3 +786,12 @@ def update(self, left=None, bottom=None, right=None, top=None,
             self.wspace = wspace
         if hspace is not None:
             self.hspace = hspace
+
+    def reset(self):
+        """Restore the positioning parameters to the default values"""
path: lib/matplotlib/gridspec.py
text: @user1:
```suggestion
        """Restore the subplot positioning parameters to the default rcParams values"""
```