From 167234c9f326f492226acc11b64901d2226d33ea Mon Sep 17 00:00:00 2001 From: cvanelteren Date: Tue, 14 Jul 2026 13:11:53 +1000 Subject: [PATCH] Fix Figure.clear leaving stale state and decouple ui kwarg routing Matplotlib's `clear()` detaches every axes and artist and sets `_suptitle` to None, but ultraplot kept its own bookkeeping pointing at the destroyed objects: a cleared figure went on handing out dead axes through `subplotgrid`, `_get_subplot`, `_iter_subplots` and `_iter_axes`, kept a stale gridspec and label counter, leaked figure-level panels, and raised `AttributeError` from the next `format(suptitle=...)` because the suptitle artist was gone, so this adds `SubplotManager.reset()` and overrides `Figure.clear()` to call it, empty the panel dict, reset the layout flags, and rebuild the label artists via the extracted `_init_super_labels()` (which also covers `clf()`, matplotlib's alias for `clear()`). It further removes a footgun in `ui.py`, which split figure keywords from subplot keywords by introspecting the signatures of `Figure._parse_proj` and `Figure._add_subplots` even though those are pure pass-throughs whose only remaining job was to mirror `SubplotManager`'s parameter list -- a contract nothing enforced, and one that already fired once when collapsing `_parse_proj` to `(*args, **kwargs)` made `_pop_params` see no projection parameters and silently routed `proj` to the figure, raising from `Figure.set()`; `_pop_params` now introspects `SubplotManager.parse_proj` and `.add_subplots`, which actually own those parameters, so the delegators are free to collapse back to `(*args, **kwargs)`. Closes #755, closes #756 --- ultraplot/_subplots.py | 12 +++ ultraplot/figure.py | 100 ++++++++++-------------- ultraplot/tests/test_figure.py | 77 ++++++++++++++++++ ultraplot/tests/test_subplot_manager.py | 41 ++++++++++ ultraplot/ui.py | 8 +- 5 files changed, 178 insertions(+), 60 deletions(-) diff --git a/ultraplot/_subplots.py b/ultraplot/_subplots.py index b33d7d40b..b0594973b 100644 --- a/ultraplot/_subplots.py +++ b/ultraplot/_subplots.py @@ -36,6 +36,18 @@ def __init__(self, figure: "Figure"): self.counter: int = 0 self._gridspec = None + def reset(self): + """ + Forget every subplot and release the gridspec. + + Called by `~ultraplot.figure.Figure.clear`, which destroys the axes this + manager tracks. Without this the figure keeps handing out axes that are no + longer attached to it. + """ + self.subplot_dict.clear() + self.counter = 0 + self._gridspec = None + @property def gridspec(self): """The single GridSpec used for all subplots in the figure.""" diff --git a/ultraplot/figure.py b/ultraplot/figure.py index cef33f2ad..0a8105640 100644 --- a/ultraplot/figure.py +++ b/ultraplot/figure.py @@ -1034,7 +1034,19 @@ def _init_figure_state(self, figwidth, figheight, kwargs): with self._context_authorized(): super().__init__(**kwargs) - # Super labels + self._init_super_labels() + + # Apply initial formatting (ignores user-input rc_mode) + self.format(rc_kw=rc_kw, rc_mode=1, skip_axes=True, **kw_format) + + def _init_super_labels(self): + """ + Create the figure-level label artists and their style state. + + NOTE: Also called by `clear`, which discards every artist on the figure and + sets ``_suptitle`` to None. The labels must be rebuilt there or the next + ``format(suptitle=...)`` raises on the missing artist. + """ self._suptitle = self.text(0.5, 0.95, "", ha="center", va="bottom") self._supxlabel_dict = {} self._supylabel_dict = {} @@ -1053,8 +1065,30 @@ def _init_figure_state(self, figwidth, figheight, kwargs): d["bottom"] = rc["bottomlabel.pad"] d["top"] = rc["toplabel.pad"] - # Apply initial formatting (ignores user-input rc_mode) - self.format(rc_kw=rc_kw, rc_mode=1, skip_axes=True, **kw_format) + @_clear_border_cache + def clear(self, keep_observers=False): + """ + Clear the figure, discarding all subplots, panels, and figure-level labels. + + Parameters + ---------- + keep_observers : bool, default: False + Whether to retain the figure's observers, e.g. a GUI widget tracking + the axes. + + See also + -------- + matplotlib.figure.Figure.clear + """ + # Matplotlib removes every axes and artist, so the ultraplot state that + # points at them is now dangling. Rebuild it rather than leaving the figure + # handing out axes it no longer owns. + super().clear(keep_observers=keep_observers) + self._subplots.reset() + self._panel_dict = {"left": [], "right": [], "bottom": [], "top": []} + self._layout_initialized = False + self._layout_dirty = True + self._init_super_labels() @override def draw(self, renderer): @@ -1700,34 +1734,9 @@ def _parse_backend(backend=None, basemap=None): """Delegate to SubplotManager.""" return SubplotManager.parse_backend(backend, basemap) - def _parse_proj( - self, - proj=None, - projection=None, - proj_kw=None, - projection_kw=None, - backend=None, - basemap=None, - **kwargs, - ): - """ - Delegate to SubplotManager. - - NOTE: The parameters must stay spelled out rather than collapsing into - ``**kwargs``. `~ultraplot.ui.subplot` uses ``_pop_params`` to introspect - this signature and decide which keywords belong to the subplot instead of - the figure; a ``*args, **kwargs`` signature silently routes them to the - figure and raises on ``Figure.set()``. - """ - return self._subplots.parse_proj( - proj=proj, - projection=projection, - proj_kw=proj_kw, - projection_kw=projection_kw, - backend=backend, - basemap=basemap, - **kwargs, - ) + def _parse_proj(self, *args, **kwargs): + """Delegate to SubplotManager.""" + return self._subplots.parse_proj(*args, **kwargs) def _get_align_axes(self, side): """ @@ -2233,34 +2242,9 @@ def get_key(ax): else: ref._shared_axes[which].join(ref, other) - def _add_subplots( - self, - array=None, - nrows=1, - ncols=1, - order="C", - proj=None, - projection=None, - proj_kw=None, - projection_kw=None, - backend=None, - basemap=None, - **kwargs, - ): + def _add_subplots(self, *args, **kwargs): """Delegate to SubplotManager.""" - return self._subplots.add_subplots( - array=array, - nrows=nrows, - ncols=ncols, - order=order, - proj=proj, - projection=projection, - proj_kw=proj_kw, - projection_kw=projection_kw, - backend=backend, - basemap=basemap, - **kwargs, - ) + return self._subplots.add_subplots(*args, **kwargs) def _align_axis_label(self, x): """ diff --git a/ultraplot/tests/test_figure.py b/ultraplot/tests/test_figure.py index 6ae668c4f..c8a006cf7 100644 --- a/ultraplot/tests/test_figure.py +++ b/ultraplot/tests/test_figure.py @@ -920,3 +920,80 @@ def test_refaspect_as_tuple(): fig, axs = uplt.subplots(refaspect=(16, 9)) fig.canvas.draw() uplt.close(fig) + + +def test_clear_drops_subplot_state(): + """ + clear() must forget the subplots it destroyed. Otherwise the figure keeps + handing out axes that matplotlib already detached from it. + """ + fig, axs = uplt.subplots(nrows=1, ncols=2) + fig.clear() + assert fig.axes == [] + assert len(fig.subplotgrid) == 0 + assert fig._get_subplot(1) is None + assert list(fig._iter_subplots()) == [] + assert fig.gridspec is None + uplt.close(fig) + + +def test_clear_drops_panel_state(): + """clear() also forgets figure-level panels created by e.g. fig.colorbar.""" + fig, axs = uplt.subplots(nrows=1, ncols=2) + m = axs[0].pcolormesh(np.arange(16).reshape(4, 4)) + fig.colorbar(m, loc="r") + assert fig._panel_dict["right"] + fig.clear() + assert not any(fig._panel_dict.values()) + assert list(fig._iter_axes(panels=True)) == [] + uplt.close(fig) + + +def test_clear_resets_subplot_numbering(): + """ + The label counter restarts after clear(), so a reused figure numbers its + subplots from 1 rather than continuing from the destroyed ones. + """ + fig, axs = uplt.subplots(nrows=1, ncols=2) + fig.clear() + ax = fig.add_subplot(111) + assert ax.number == 1 + assert list(fig._iter_subplots()) == [ax] + uplt.close(fig) + + +def test_clear_allows_suptitle(): + """ + Matplotlib's clear() sets _suptitle to None, so ultraplot must rebuild its + label artists or the next format(suptitle=...) raises AttributeError. + """ + fig, axs = uplt.subplots(nrows=1, ncols=2) + fig.clear() + fig.add_subplot(111) + fig.format(suptitle="after clear") + fig.canvas.draw() + assert fig._suptitle.get_text() == "after clear" + assert fig._suptitle in fig.texts # detached artists never render + uplt.close(fig) + + +def test_clf_alias_clears_subplot_state(): + """clf() is matplotlib's alias for clear() and must reset the same state.""" + fig, axs = uplt.subplots(nrows=1, ncols=2) + fig.clf() + assert len(fig.subplotgrid) == 0 + assert fig.gridspec is None + uplt.close(fig) + + +def test_figure_is_reusable_after_clear(): + """A cleared figure can be drawn again from scratch.""" + fig, axs = uplt.subplots(nrows=2, ncols=2) + fig.canvas.draw() + fig.clear() + axs = fig.add_subplots(nrows=1, ncols=3) + axs[0].plot([1, 2, 3]) + fig.canvas.draw() + assert len(fig.subplotgrid) == 3 + assert fig.gridspec.get_geometry() == (1, 3) + uplt.close(fig) diff --git a/ultraplot/tests/test_subplot_manager.py b/ultraplot/tests/test_subplot_manager.py index 2503a7048..2b0aed87a 100644 --- a/ultraplot/tests/test_subplot_manager.py +++ b/ultraplot/tests/test_subplot_manager.py @@ -2,11 +2,14 @@ Tests for SubplotManager (ultraplot._subplots). """ +import inspect + import matplotlib.projections as mproj import numpy as np import pytest import ultraplot as uplt +from ultraplot import figure as pfigure from ultraplot import gridspec as pgridspec from ultraplot._subplots import SubplotManager from ultraplot.axes.container import ExternalAxesContainer @@ -363,3 +366,41 @@ def test_ui_subplots_routes_projection_kwargs(): fig, axs = uplt.subplots(nrows=1, ncols=2, proj="polar") assert all(isinstance(ax, uplt.axes.PolarAxes) for ax in axs) uplt.close(fig) + + +def test_ui_introspects_manager_not_figure_delegator(): + """ + ``ui.subplot``/``ui.subplots`` split figure keywords from subplot keywords by + introspecting the manager, which owns these parameters -- not the thin + ``Figure`` delegators, whose signatures are free to change. + + These names are therefore load-bearing: collapsing them into ``**kwargs`` + silently routes ``proj`` to the figure, which then raises from ``Figure.set()``. + """ + proj_params = set(inspect.signature(SubplotManager.parse_proj).parameters) + assert { + "proj", + "projection", + "proj_kw", + "projection_kw", + "backend", + "basemap", + } <= proj_params + + subplots_params = set(inspect.signature(SubplotManager.add_subplots).parameters) + assert {"array", "nrows", "ncols", "order"} <= subplots_params + assert {"proj", "projection", "proj_kw", "projection_kw"} <= subplots_params + + +def test_figure_delegator_signature_is_not_load_bearing(): + """ + The routing above must survive the Figure pass-throughs being collapsed -- + that collapse is exactly the regression fixed in #755. + """ + for name in ("_parse_proj", "_add_subplots"): + params = set(inspect.signature(getattr(pfigure.Figure, name)).parameters) + assert params == {"self", "args", "kwargs"} + + fig, ax = uplt.subplot(proj="polar") + assert isinstance(ax, uplt.axes.PolarAxes) + uplt.close(fig) diff --git a/ultraplot/ui.py b/ultraplot/ui.py index 1cafa496f..f61b03840 100644 --- a/ultraplot/ui.py +++ b/ultraplot/ui.py @@ -8,6 +8,7 @@ from . import axes as paxes from . import figure as pfigure from . import gridspec as pgridspec +from ._subplots import SubplotManager from .internals import ( _not_none, _pop_params, @@ -182,7 +183,10 @@ def subplot(**kwargs): _parse_figsize(kwargs) rc_kw, rc_mode = _pop_rc(kwargs) kwsub = _pop_props(kwargs, "patch") # e.g. 'color' - kwsub.update(_pop_params(kwargs, pfigure.Figure._parse_proj)) + # NOTE: Introspect the manager, which owns these parameters, rather than the + # thin Figure delegator. Pointing this at a pass-through means any cleanup of + # that pass-through's signature silently routes 'proj' to the figure instead. + kwsub.update(_pop_params(kwargs, SubplotManager.parse_proj)) for sig in paxes.Axes._format_signatures.values(): kwsub.update(_pop_params(kwargs, sig)) kwargs["aspect"] = kwsub.pop("aspect", None) # keyword conflict @@ -227,7 +231,7 @@ def subplots(*args, **kwargs): _parse_figsize(kwargs) rc_kw, rc_mode = _pop_rc(kwargs) kwsubs = _pop_props(kwargs, "patch") # e.g. 'color' - kwsubs.update(_pop_params(kwargs, pfigure.Figure._add_subplots)) + kwsubs.update(_pop_params(kwargs, SubplotManager.add_subplots)) kwsubs.update(_pop_params(kwargs, pgridspec.GridSpec._update_params)) for sig in paxes.Axes._format_signatures.values(): kwsubs.update(_pop_params(kwargs, sig))