From 6557e3083810f2de892acd82c78cd0e98a3b7e2e Mon Sep 17 00:00:00 2001 From: anon Date: Sun, 7 Jun 2026 05:49:03 +0200 Subject: [PATCH 1/2] Set panel title/aspect/frameon once per panel, not per render command (#695) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In `show()`, the title/`set_aspect`/`axis("off")` block sat inside the `for cmd, params in render_cmds:` loop, so for a chained call (e.g. render_images().render_shapes()) these ran once per render command instead of once per panel, and the title-length `IndexError` check re-evaluated each iteration (so it could surface mid-loop rather than up front). Dedent the block one level so it runs once per panel after the per-command loop. Output is unchanged (the calls are idempotent) — verified byte-identical to main for single- and multi-command chains with title and frameon set. --- src/spatialdata_plot/pl/basic.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/spatialdata_plot/pl/basic.py b/src/spatialdata_plot/pl/basic.py index f864482c..f837fe4c 100644 --- a/src/spatialdata_plot/pl/basic.py +++ b/src/spatialdata_plot/pl/basic.py @@ -1794,19 +1794,21 @@ def _draw_colorbar( colorbar_requests=axis_colorbar_requests, ) - if title is None: - t = panel_key if panel_key is not None else cs - elif len(title) == 1: - t = title[0] - else: - try: - t = title[i] - except IndexError as e: - raise IndexError("The number of titles must match the number of panels.") from e - ax.set_title(t) - ax.set_aspect("equal") - if fig_params.frameon is False: - ax.axis("off") + # Title/aspect/frameon are panel-level: set once per panel, after the + # per-command loop above (not once per render command). + if title is None: + t = panel_key if panel_key is not None else cs + elif len(title) == 1: + t = title[0] + else: + try: + t = title[i] + except IndexError as e: + raise IndexError("The number of titles must match the number of panels.") from e + ax.set_title(t) + ax.set_aspect("equal") + if fig_params.frameon is False: + ax.axis("off") if has_shapes and wants_shapes: empty_shape_elements = [ From 03a3c1ccbc6f42497103a015ef1d956b8554e300 Mon Sep 17 00:00:00 2001 From: anon Date: Sun, 14 Jun 2026 18:55:26 +0200 Subject: [PATCH 2/2] feat(show): validate title count up front Builds on the per-panel title fix (#695): before the panel loop, validate that `title`, when a list, has length 1 or exactly num_panels. This: - surfaces the error before any drawing (not mid-render), - rejects an over-long title list (previously silently truncated), and - makes the per-panel IndexError dead code, so the try/except is removed. The error is now a ValueError (the right type for an invalid argument) raised once up front, instead of an IndexError raised per panel mid-render. No test pinned the old exception type. Adds non-visual regression tests for single- and multi-panel title-count validation. Pre-existing mypy error in utils.py (_resolve_measure_table returning Any) is unrelated and unchanged here; it is fixed separately in #714. --- src/spatialdata_plot/pl/basic.py | 17 +++++++++++------ tests/pl/test_show.py | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/spatialdata_plot/pl/basic.py b/src/spatialdata_plot/pl/basic.py index f837fe4c..7bb6b4b4 100644 --- a/src/spatialdata_plot/pl/basic.py +++ b/src/spatialdata_plot/pl/basic.py @@ -1519,6 +1519,14 @@ def show( panels = [(cs, None) for cs in coordinate_systems] num_panels = len(panels) + # Titles are panel-level: require one title (broadcast to all panels) or exactly one + # per panel. Validating up front surfaces the error before any drawing, and also rejects + # an over-long list (previously silently truncated). + if title is not None and len(title) not in (1, num_panels): + raise ValueError( + f"The number of titles ({len(title)}) must be 1 or match the number of panels ({num_panels})." + ) + if ax is not None: n_ax = 1 if isinstance(ax, Axes) else len(ax) if num_panels != n_ax: @@ -1794,17 +1802,14 @@ def _draw_colorbar( colorbar_requests=axis_colorbar_requests, ) - # Title/aspect/frameon are panel-level: set once per panel, after the - # per-command loop above (not once per render command). + # Title/aspect/frameon are panel-level: set once per panel. if title is None: t = panel_key if panel_key is not None else cs elif len(title) == 1: t = title[0] else: - try: - t = title[i] - except IndexError as e: - raise IndexError("The number of titles must match the number of panels.") from e + # len(title) == num_panels is guaranteed by the up-front check above. + t = title[i] ax.set_title(t) ax.set_aspect("equal") if fig_params.frameon is False: diff --git a/tests/pl/test_show.py b/tests/pl/test_show.py index 949a9237..47fade9a 100644 --- a/tests/pl/test_show.py +++ b/tests/pl/test_show.py @@ -136,6 +136,24 @@ def test_fig_parameter_default_no_warning(sdata_blobs: SpatialData): plt.close("all") +def test_title_count_validation(sdata_blobs: SpatialData): + """title must be length 1 or one-per-panel; mismatches raise up front (regression for #695).""" + base = sdata_blobs.pl.render_images(element="blobs_image") + with pytest.raises(ValueError, match="number of titles"): # single panel, too many + base.pl.show(title=["a", "b"], show=False) + plt.close("all") + + set_transformation(sdata_blobs["blobs_image"], Identity(), "second_cs") + base2 = sdata_blobs.pl.render_images(element="blobs_image") + with pytest.raises(ValueError, match="number of titles"): # 2 panels, too many (was silently truncated) + base2.pl.show(title=["a", "b", "c"], show=False) + plt.close("all") + + axs = base2.pl.show(title=["left", "right"], return_ax=True, show=False) # one per panel -> applied + assert sorted(a.get_title() for a in axs) == ["left", "right"] + plt.close("all") + + def test_fig_parameter_warns_with_ax_list(sdata_blobs: SpatialData): """Passing fig= alongside a list of axes should also emit the deprecation (regression for #625).""" set_transformation(sdata_blobs["blobs_image"], Identity(), "second_cs")