Set panel title/aspect/frameon once per panel, not per render command - #702

Merged
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel
Jun 14, 2026
Merged

Set panel title/aspect/frameon once per panel, not per render command#702
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel

Conversation

@timtreis

Copy link
Copy Markdown
Member

Summary

Fixes#695. In PlotAccessor.show, the panel title / set_aspect("equal") / axis("off") block sat inside the for 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-length IndexError check re-evaluated each iteration — so a mismatched title length 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

  • Byte-identical to main (RGBA buffers compared via np.array_equal) for a single-command plot and a 3-command chain with title= and frameon=False set — the calls are idempotent, so output is unchanged.
  • Non-visual suite: 368 passed, 1 skipped.
  • pre-commit: ruff + mypy pass.

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_panels once before the panel loop. That's left out here to keep the diff minimal — the per-panel IndexError still fires (now once per panel). Happy to add the up-front validation if preferred.

…#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
timtreisforce-pushed the fix/issue-695-title-per-panel branch 2 times, most recently from b05fb5b to 6bce242CompareJune 14, 2026 16:59
@codecov-commenter

codecov-commenter commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.39%. Comparing base (b370b1f) to head (03a3c1c).
⚠️ Report is 1 commits behind head on main.

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 
Files with missing linesCoverage Δ
src/spatialdata_plot/pl/basic.py80.82% <100.00%> (+1.40%)⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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
timtreisforce-pushed the fix/issue-695-title-per-panel branch from 6bce242 to 03a3c1cCompareJune 14, 2026 17:10
@timtreis
timtreis merged commit cf0073a into mainJun 14, 2026
7 of 8 checks passed
@timtreis
timtreis deleted the fix/issue-695-title-per-panel branch June 14, 2026 17:17
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

show(): title/aspect/frameon set once per render-command instead of once per panel

2 participants

@timtreis@codecov-commenter
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Set panel title/aspect/frameon once per panel, not per render command - #702

Merged
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel
Jun 14, 2026
Merged

Set panel title/aspect/frameon once per panel, not per render command#702
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel

Conversation

@timtreis

Copy link
Copy Markdown
Member

Summary

Fixes#695. In PlotAccessor.show, the panel title / set_aspect("equal") / axis("off") block sat inside the for 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-length IndexError check re-evaluated each iteration — so a mismatched title length 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

  • Byte-identical to main (RGBA buffers compared via np.array_equal) for a single-command plot and a 3-command chain with title= and frameon=False set — the calls are idempotent, so output is unchanged.
  • Non-visual suite: 368 passed, 1 skipped.
  • pre-commit: ruff + mypy pass.

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_panels once before the panel loop. That's left out here to keep the diff minimal — the per-panel IndexError still fires (now once per panel). Happy to add the up-front validation if preferred.

…#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
timtreisforce-pushed the fix/issue-695-title-per-panel branch 2 times, most recently from b05fb5b to 6bce242CompareJune 14, 2026 16:59
@codecov-commenter

codecov-commenter commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.39%. Comparing base (b370b1f) to head (03a3c1c).
⚠️ Report is 1 commits behind head on main.

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 
Files with missing linesCoverage Δ
src/spatialdata_plot/pl/basic.py80.82% <100.00%> (+1.40%)⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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
timtreisforce-pushed the fix/issue-695-title-per-panel branch from 6bce242 to 03a3c1cCompareJune 14, 2026 17:10
@timtreis
timtreis merged commit cf0073a into mainJun 14, 2026
7 of 8 checks passed
@timtreis
timtreis deleted the fix/issue-695-title-per-panel branch June 14, 2026 17:17
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

show(): title/aspect/frameon set once per render-command instead of once per panel

2 participants

@timtreis@codecov-commenter
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Set panel title/aspect/frameon once per panel, not per render command - #702

Merged
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel
Jun 14, 2026
Merged

Set panel title/aspect/frameon once per panel, not per render command#702
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel

Conversation

@timtreis

Copy link
Copy Markdown
Member

Summary

Fixes#695. In PlotAccessor.show, the panel title / set_aspect("equal") / axis("off") block sat inside the for 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-length IndexError check re-evaluated each iteration — so a mismatched title length 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

  • Byte-identical to main (RGBA buffers compared via np.array_equal) for a single-command plot and a 3-command chain with title= and frameon=False set — the calls are idempotent, so output is unchanged.
  • Non-visual suite: 368 passed, 1 skipped.
  • pre-commit: ruff + mypy pass.

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_panels once before the panel loop. That's left out here to keep the diff minimal — the per-panel IndexError still fires (now once per panel). Happy to add the up-front validation if preferred.

…#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
timtreisforce-pushed the fix/issue-695-title-per-panel branch 2 times, most recently from b05fb5b to 6bce242CompareJune 14, 2026 16:59
@codecov-commenter

codecov-commenter commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.39%. Comparing base (b370b1f) to head (03a3c1c).
⚠️ Report is 1 commits behind head on main.

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 
Files with missing linesCoverage Δ
src/spatialdata_plot/pl/basic.py80.82% <100.00%> (+1.40%)⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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
timtreisforce-pushed the fix/issue-695-title-per-panel branch from 6bce242 to 03a3c1cCompareJune 14, 2026 17:10
@timtreis
timtreis merged commit cf0073a into mainJun 14, 2026
7 of 8 checks passed
@timtreis
timtreis deleted the fix/issue-695-title-per-panel branch June 14, 2026 17:17
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

show(): title/aspect/frameon set once per render-command instead of once per panel

2 participants

@timtreis@codecov-commenter
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Set panel title/aspect/frameon once per panel, not per render command - #702

Merged
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel
Jun 14, 2026
Merged

Set panel title/aspect/frameon once per panel, not per render command#702
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel

Conversation

@timtreis

Copy link
Copy Markdown
Member

Summary

Fixes#695. In PlotAccessor.show, the panel title / set_aspect("equal") / axis("off") block sat inside the for 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-length IndexError check re-evaluated each iteration — so a mismatched title length 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

  • Byte-identical to main (RGBA buffers compared via np.array_equal) for a single-command plot and a 3-command chain with title= and frameon=False set — the calls are idempotent, so output is unchanged.
  • Non-visual suite: 368 passed, 1 skipped.
  • pre-commit: ruff + mypy pass.

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_panels once before the panel loop. That's left out here to keep the diff minimal — the per-panel IndexError still fires (now once per panel). Happy to add the up-front validation if preferred.

…#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
timtreisforce-pushed the fix/issue-695-title-per-panel branch 2 times, most recently from b05fb5b to 6bce242CompareJune 14, 2026 16:59
@codecov-commenter

codecov-commenter commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.39%. Comparing base (b370b1f) to head (03a3c1c).
⚠️ Report is 1 commits behind head on main.

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 
Files with missing linesCoverage Δ
src/spatialdata_plot/pl/basic.py80.82% <100.00%> (+1.40%)⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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
timtreisforce-pushed the fix/issue-695-title-per-panel branch from 6bce242 to 03a3c1cCompareJune 14, 2026 17:10
@timtreis
timtreis merged commit cf0073a into mainJun 14, 2026
7 of 8 checks passed
@timtreis
timtreis deleted the fix/issue-695-title-per-panel branch June 14, 2026 17:17
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

show(): title/aspect/frameon set once per render-command instead of once per panel

2 participants

@timtreis@codecov-commenter
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Set panel title/aspect/frameon once per panel, not per render command - #702

Merged
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel
Jun 14, 2026
Merged

Set panel title/aspect/frameon once per panel, not per render command#702
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel

Conversation

@timtreis

Copy link
Copy Markdown
Member

Summary

Fixes#695. In PlotAccessor.show, the panel title / set_aspect("equal") / axis("off") block sat inside the for 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-length IndexError check re-evaluated each iteration — so a mismatched title length 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

  • Byte-identical to main (RGBA buffers compared via np.array_equal) for a single-command plot and a 3-command chain with title= and frameon=False set — the calls are idempotent, so output is unchanged.
  • Non-visual suite: 368 passed, 1 skipped.
  • pre-commit: ruff + mypy pass.

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_panels once before the panel loop. That's left out here to keep the diff minimal — the per-panel IndexError still fires (now once per panel). Happy to add the up-front validation if preferred.

…#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
timtreisforce-pushed the fix/issue-695-title-per-panel branch 2 times, most recently from b05fb5b to 6bce242CompareJune 14, 2026 16:59
@codecov-commenter

codecov-commenter commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.39%. Comparing base (b370b1f) to head (03a3c1c).
⚠️ Report is 1 commits behind head on main.

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 
Files with missing linesCoverage Δ
src/spatialdata_plot/pl/basic.py80.82% <100.00%> (+1.40%)⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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
timtreisforce-pushed the fix/issue-695-title-per-panel branch from 6bce242 to 03a3c1cCompareJune 14, 2026 17:10
@timtreis
timtreis merged commit cf0073a into mainJun 14, 2026
7 of 8 checks passed
@timtreis
timtreis deleted the fix/issue-695-title-per-panel branch June 14, 2026 17:17
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

show(): title/aspect/frameon set once per render-command instead of once per panel

2 participants

@timtreis@codecov-commenter
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Set panel title/aspect/frameon once per panel, not per render command - #702

Merged
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel
Jun 14, 2026
Merged

Set panel title/aspect/frameon once per panel, not per render command#702
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel

Conversation

@timtreis

Copy link
Copy Markdown
Member

Summary

Fixes#695. In PlotAccessor.show, the panel title / set_aspect("equal") / axis("off") block sat inside the for 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-length IndexError check re-evaluated each iteration — so a mismatched title length 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

  • Byte-identical to main (RGBA buffers compared via np.array_equal) for a single-command plot and a 3-command chain with title= and frameon=False set — the calls are idempotent, so output is unchanged.
  • Non-visual suite: 368 passed, 1 skipped.
  • pre-commit: ruff + mypy pass.

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_panels once before the panel loop. That's left out here to keep the diff minimal — the per-panel IndexError still fires (now once per panel). Happy to add the up-front validation if preferred.

…#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
timtreisforce-pushed the fix/issue-695-title-per-panel branch 2 times, most recently from b05fb5b to 6bce242CompareJune 14, 2026 16:59
@codecov-commenter

codecov-commenter commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.39%. Comparing base (b370b1f) to head (03a3c1c).
⚠️ Report is 1 commits behind head on main.

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 
Files with missing linesCoverage Δ
src/spatialdata_plot/pl/basic.py80.82% <100.00%> (+1.40%)⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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
timtreisforce-pushed the fix/issue-695-title-per-panel branch from 6bce242 to 03a3c1cCompareJune 14, 2026 17:10
@timtreis
timtreis merged commit cf0073a into mainJun 14, 2026
7 of 8 checks passed
@timtreis
timtreis deleted the fix/issue-695-title-per-panel branch June 14, 2026 17:17
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

show(): title/aspect/frameon set once per render-command instead of once per panel

2 participants

@timtreis@codecov-commenter
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Set panel title/aspect/frameon once per panel, not per render command - #702

Merged
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel
Jun 14, 2026
Merged

Set panel title/aspect/frameon once per panel, not per render command#702
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel

Conversation

@timtreis

Copy link
Copy Markdown
Member

Summary

Fixes#695. In PlotAccessor.show, the panel title / set_aspect("equal") / axis("off") block sat inside the for 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-length IndexError check re-evaluated each iteration — so a mismatched title length 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

  • Byte-identical to main (RGBA buffers compared via np.array_equal) for a single-command plot and a 3-command chain with title= and frameon=False set — the calls are idempotent, so output is unchanged.
  • Non-visual suite: 368 passed, 1 skipped.
  • pre-commit: ruff + mypy pass.

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_panels once before the panel loop. That's left out here to keep the diff minimal — the per-panel IndexError still fires (now once per panel). Happy to add the up-front validation if preferred.

…#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
timtreisforce-pushed the fix/issue-695-title-per-panel branch 2 times, most recently from b05fb5b to 6bce242CompareJune 14, 2026 16:59
@codecov-commenter

codecov-commenter commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.39%. Comparing base (b370b1f) to head (03a3c1c).
⚠️ Report is 1 commits behind head on main.

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 
Files with missing linesCoverage Δ
src/spatialdata_plot/pl/basic.py80.82% <100.00%> (+1.40%)⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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
timtreisforce-pushed the fix/issue-695-title-per-panel branch from 6bce242 to 03a3c1cCompareJune 14, 2026 17:10
@timtreis
timtreis merged commit cf0073a into mainJun 14, 2026
7 of 8 checks passed
@timtreis
timtreis deleted the fix/issue-695-title-per-panel branch June 14, 2026 17:17
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

show(): title/aspect/frameon set once per render-command instead of once per panel

2 participants

@timtreis@codecov-commenter
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Set panel title/aspect/frameon once per panel, not per render command - #702

Merged
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel
Jun 14, 2026
Merged

Set panel title/aspect/frameon once per panel, not per render command#702
timtreis merged 2 commits into
mainfrom
fix/issue-695-title-per-panel

Conversation

@timtreis

Copy link
Copy Markdown
Member

Summary

Fixes#695. In PlotAccessor.show, the panel title / set_aspect("equal") / axis("off") block sat inside the for 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-length IndexError check re-evaluated each iteration — so a mismatched title length 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

  • Byte-identical to main (RGBA buffers compared via np.array_equal) for a single-command plot and a 3-command chain with title= and frameon=False set — the calls are idempotent, so output is unchanged.
  • Non-visual suite: 368 passed, 1 skipped.
  • pre-commit: ruff + mypy pass.

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_panels once before the panel loop. That's left out here to keep the diff minimal — the per-panel IndexError still fires (now once per panel). Happy to add the up-front validation if preferred.

…#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
timtreisforce-pushed the fix/issue-695-title-per-panel branch 2 times, most recently from b05fb5b to 6bce242CompareJune 14, 2026 16:59
@codecov-commenter

codecov-commenter commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.39%. Comparing base (b370b1f) to head (03a3c1c).
⚠️ Report is 1 commits behind head on main.

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 
Files with missing linesCoverage Δ
src/spatialdata_plot/pl/basic.py80.82% <100.00%> (+1.40%)⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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
timtreisforce-pushed the fix/issue-695-title-per-panel branch from 6bce242 to 03a3c1cCompareJune 14, 2026 17:10
@timtreis
timtreis merged commit cf0073a into mainJun 14, 2026
7 of 8 checks passed
@timtreis
timtreis deleted the fix/issue-695-title-per-panel branch June 14, 2026 17:17
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

show(): title/aspect/frameon set once per render-command instead of once per panel

2 participants

@timtreis@codecov-commenter