Set panel title/aspect/frameon once per panel, not per render command#702
Merged
Conversation
…#695) 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.
timtreis
force-pushed
the
fix/issue-695-title-per-panel
branch
2 times, most recently
from
b05fb5b to
6bce242
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #702 +/- ##
==========================================
+ Coverage 76.28% 76.39% +0.11%
==========================================
Files 14 14
Lines 4327 4326 -1
Branches 1006 1007 +1
==========================================
+ Hits 3301 3305 +4
+ Misses 667 663 -4
+ Partials 359 358 -1
🚀 New features to boost your workflow:
|
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.
timtreis
force-pushed
the
fix/issue-695-title-per-panel
branch
from
6bce242 to
03a3c1c
Compare
timtreis
added a commit
that referenced
this pull request
Jun 14, 2026
Integrate #702 (per-panel title fix) and #703 (as_points + fast extent) into the show() decomposition. Conflict resolution in src/spatialdata_plot/pl/basic.py: - #702 up-front title-count validation: kept (after num_panels). The adjacent axes/panel-count check was dropped here because the decomposition already relocated it into _plan_panels. - #702 simplified title selection (dropped the per-panel try/except): applied to the extracted _finalize_panel helper. - #703 had no show()-level render-dispatch changes (as_points is param-driven in render.py, handled inside _render_panel already); its only show()-level change, get_extent -> _get_extent_fast, auto-merged into the extent block and consumes _render_panel's `wants` dict. Also sweeps up two pre-existing #703 lint/type nits in utils.py surfaced by the merge: _fast_extent docstring (ruff D205) and _get_extent_fast Any-return (mypy). Verified: ruff + mypy clean; 109 non-visual show/shapes/labels tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #695. In
PlotAccessor.show, the panel title /set_aspect("equal")/axis("off")block sat inside thefor cmd, params in render_cmds:loop (basic.py:~1797), so it ran once per render command instead of once per panel.For a chained call like
render_images().pl.render_shapes(...),ax.set_title(...)/set_aspect/axis("off")executed N times per panel, and the title-lengthIndexErrorcheck re-evaluated each iteration — so a mismatchedtitlelength could surface mid-render rather than up front.Fix
Dedent the block one level so it runs once per panel, after the per-command loop. That's the entire change — Python's indentation moves it out of the inner loop; it already uses only panel-level locals (
title,panel_key,cs,i,ax,fig_params).Verification
main(RGBA buffers compared vianp.array_equal) for a single-command plot and a 3-command chain withtitle=andframeon=Falseset — the calls are idempotent, so output is unchanged.No regression test added: the change removes redundant repeated calls with no observable output difference, so there's no new behavior to lock that the existing multi-panel-title visual tests (CI) don't already cover.
Note / possible follow-up
The issue also suggested validating
len(title) == num_panelsonce before the panel loop. That's left out here to keep the diff minimal — the per-panelIndexErrorstill fires (now once per panel). Happy to add the up-front validation if preferred.