From ab0f3cbce71e7000be2221f0eff8c3580eee12f5 Mon Sep 17 00:00:00 2001 From: anon Date: Fri, 9 Oct 2026 16:33:21 +0200 Subject: [PATCH] fix(show): dedup redundant coordinate systems for single-axes render (#749) When an element carries transformations to several coordinate systems that render the same elements (e.g. visium's "" and "_downscaled_lowres"; filter_by_coordinate_system can't strip the extra transform, upstream #176), auto-detection produced one panel per coordinate system and `show(ax=single_ax)` raised a "Mismatch between number of matplotlib axes objects and number of panels" ValueError. Deduplicate auto-detected coordinate systems by their renderable-element set when axes are supplied: keep the first of each distinct set so a single axes is satisfied, and warn naming the dropped duplicates (pointing to `coordinate_systems=`). Genuinely distinct coordinate systems keep more sets than axes and still raise the mismatch error. Scoped to the ax-provided, auto-detected path, so the no-ax multi-panel behaviour is unchanged. --- plans/issue-749.md | 74 ++++++++++++++++++++++++++++++++ src/spatialdata_plot/pl/basic.py | 31 +++++++++++++ tests/pl/test_render.py | 36 ++++++++++++++-- 3 files changed, 137 insertions(+), 4 deletions(-) create mode 100644 plans/issue-749.md diff --git a/plans/issue-749.md b/plans/issue-749.md new file mode 100644 index 00000000..5fcf5c15 --- /dev/null +++ b/plans/issue-749.md @@ -0,0 +1,74 @@ +# Issue #749: `pl.show()` ValueError — axes/panels count mismatch + +## Root cause + +When a SpatialElement carries transformations to **multiple coordinate systems** +(e.g. visium's `""` and `"_downscaled_lowres"` pair), `filter_by_coordinate_system` +cannot strip the extra transformation (upstream spatialdata #176). `show()` then +auto-detects **more coordinate systems than the user intended**. When the user +passes a single `ax`, `_plan_panels` raises: + +``` +ValueError: Mismatch between number of matplotlib axes objects (1) and number of panels (2). +``` + +PR #580 added a `strict_cs` narrowing (keep only CS with element types for *all* +render commands), but it does **not** help here: both coordinate systems contain +the *same* element, so both survive the filter. The two panels are redundant — +they render the identical element set, differing only by a scale transform. + +Confirmed reproducible on current `main` (see `/tmp/repro_issue_749.py`). + +## Proposed changes (recommended: Approach 1 — scope to the erroring path) + +Extend the existing `ax is not None and cs_was_auto` block in +`_resolve_coordinate_systems` (`src/spatialdata_plot/pl/basic.py`): after the +`strict_cs` step, if `len(coordinate_systems) > n_ax`, **deduplicate coordinate +systems by their renderable-element set** (via `_get_elements_to_be_rendered`), +keeping the first representative of each distinct set. If that brings the count +down to `<= n_ax`, use the deduplicated list and emit a `UserWarning` naming the +dropped (redundant) coordinate systems and pointing to `coordinate_systems=`. +If the sets are genuinely distinct (count still `> n_ax`), fall through to the +existing, correct `ValueError`. + +| File | Change | Rationale | +|------|--------|-----------| +| `src/spatialdata_plot/pl/basic.py` (`_resolve_coordinate_systems`) | After `strict_cs`, dedup redundant CS by element set when `ax` given + auto-detected; warn | Fixes the mismatch only on the path that currently errors → strictly backward compatible | +| `tests/pl/test_show.py` | Regression test: multi-CS element + single `ax` no longer raises; distinct-CS case still raises | Lock behavior | + +Why scope to the `ax`-provided path only: the no-`ax` case currently produces +one panel per coordinate system (redundant but not an error). Collapsing that +too would change existing multi-panel output / baselines — a backward-compat +break the reporter explicitly asked to avoid. The `ax` path currently *errors*, +so fixing it breaks nothing that worked before. + +## Edge cases +- [ ] 2 CS, same element set, single `ax` → collapse to 1, warn, render (main fix). +- [ ] 2 CS, **different** element sets, single `ax` → still raise ValueError (correct; genuinely 2 panels). +- [ ] N CS redundant, `ax` is a list of N → counts already match, no change. +- [ ] N CS redundant, `ax` list shorter than distinct-set count → raise (correct). +- [ ] `coordinate_systems=` passed explicitly → `cs_was_auto` False, block skipped, unchanged. +- [ ] No `ax` → block skipped, unchanged (still multi-panel). +- [ ] Multi-panel `color=[...]` → single CS required already; unaffected. + +## Downstream impact +- Public API unchanged (no new/changed kwargs). +- Only converts a previously-raised `ValueError` into a successful render + warning. +- No baseline image changes expected (new path only exercised by the regression test, which is non-visual). + +## Test plan +- [ ] Regression test (non-visual): multi-CS element, `filter_by_coordinate_system`, `render_shapes().show(ax=ax)` → no raise; assert 1 axis used. +- [ ] Negative test: 2 CS with different elements + single `ax` → still raises `ValueError`. +- [ ] Assert a `UserWarning` is emitted mentioning the dropped CS. + +## Risks +- Representative choice is "first in coordinate-system order", which may be the + `_downscaled_lowres` variant. Visually identical for a standalone plot (axes + autoscale), but a user overlaying multiple `show()` calls on one `ax` should + still pass `coordinate_systems=`. The warning makes the choice explicit. +- Low risk overall: new code runs only where the code previously raised. + +## Alternative (Approach 2 — not recommended) +Deduplicate redundant coordinate systems in auto-detect for **both** `ax` and +no-`ax` paths. More internally consistent, but changes existing no-`ax` +multi-panel output and possibly visual baselines → backward-incompatible. diff --git a/src/spatialdata_plot/pl/basic.py b/src/spatialdata_plot/pl/basic.py index df0312e5..8653a8a6 100644 --- a/src/spatialdata_plot/pl/basic.py +++ b/src/spatialdata_plot/pl/basic.py @@ -1790,6 +1790,37 @@ def _resolve_coordinate_systems( if strict_cs: coordinate_systems = strict_cs + # strict_cs above handles the *subset* case (keep the CS that renders all element types); + # this complementary step handles the *duplicate* case. If CS still outnumber the axes + # because an element carries transformations to several coordinate systems that render the + # *same* elements (e.g. visium's "" and "_downscaled_lowres"; upstream #176), those + # extra panels are redundant. Keep the first coordinate system of each distinct + # renderable-element set so a single axes can be satisfied, and tell the user which ones + # were dropped. Genuinely distinct coordinate systems keep more sets than axes and fall + # through to the mismatch error in _plan_panels. + if len(coordinate_systems) > n_ax: + seen_element_sets: set[frozenset[str]] = set() + deduped: list[str] = [] + dropped: list[str] = [] + for cs_name in coordinate_systems: + element_set = frozenset(_get_elements_to_be_rendered(render_cmds, cs_index, cs_name)) + if element_set in seen_element_sets: + dropped.append(cs_name) + else: + seen_element_sets.add(element_set) + deduped.append(cs_name) + # Entering under len(coordinate_systems) > n_ax, so len(deduped) <= n_ax implies + # deduped shrank, i.e. `dropped` is non-empty. + if len(deduped) <= n_ax: + warnings.warn( + f"Element(s) render identically in coordinate systems {dropped} as in " + f"{deduped}; rendering {deduped} on the provided axes and dropping the " + "redundant duplicate(s). Pass `coordinate_systems=` to choose explicitly.", + UserWarning, + stacklevel=2, + ) + coordinate_systems = deduped + return coordinate_systems diff --git a/tests/pl/test_render.py b/tests/pl/test_render.py index 29dd36b1..070f8e1d 100644 --- a/tests/pl/test_render.py +++ b/tests/pl/test_render.py @@ -97,12 +97,40 @@ def test_single_ax_explicit_multi_cs_raises(sdata_multi_cs): sdata_multi_cs.pl.render_shapes("shp").pl.show(ax=ax, coordinate_systems=["aligned", "global"]) -def test_single_ax_auto_cs_unresolvable_raises(sdata_multi_cs): - """When strict filtering can't resolve the mismatch, error includes hint.""" +def test_single_ax_auto_cs_redundant_duplicates_resolved(sdata_multi_cs): + # Regression test for #749: when an element has transformations to multiple coordinate + # systems that render the *same* elements, a single ax should render one of them (dropping + # the redundant duplicate) with a warning, instead of raising a mismatch error. _, ax = plt.subplots(1, 1) - with pytest.raises(ValueError, match="coordinate_systems="): - # Only render shapes (present in both CS), so strict filter can't narrow down + with pytest.warns(UserWarning, match="render identically"): + # "shp" is present in both "aligned" and "global" with identical content. sdata_multi_cs.pl.render_shapes("shp").pl.show(ax=ax) + assert ax.get_title() in ("aligned", "global") + + +def test_single_ax_auto_cs_distinct_elements_raises(): + # Regression test for #749: the dedup only collapses coordinate systems that render the + # *same* elements. When the detected coordinate systems render genuinely different content + # (an image-only CS and a shapes-only CS), they are distinct panels, so a single ax must + # still raise the mismatch error. + from geopandas import GeoDataFrame + from shapely.geometry import Point + from spatialdata.models import ShapesModel + + image = Image2DModel.parse( + np.zeros((1, 10, 10)), + dims=("c", "y", "x"), + transformations={"aligned": Identity()}, + ) + shp = ShapesModel.parse( + GeoDataFrame(geometry=[Point(5, 5)], data={"radius": [2]}), + transformations={"global": Identity()}, + ) + sdata = SpatialData(images={"img": image}, shapes={"shp": shp}) + + _, ax = plt.subplots(1, 1) + with pytest.raises(ValueError, match="coordinate_systems="): + sdata.pl.render_images("img").pl.render_shapes("shp").pl.show(ax=ax) def test_cs_name_with_apostrophe_does_not_crash():