Skip to content

refactor: collapse duplicated orchestration across the _render_* quartet#717

Merged
timtreis merged 1 commit into
mainfrom
refactor/issue-698-render-quartet
Jun 14, 2026
Merged

refactor: collapse duplicated orchestration across the _render_* quartet#717
timtreis merged 1 commit into
mainfrom
refactor/issue-698-render-quartet

Conversation

@timtreis

Copy link
Copy Markdown
Member

Closes #698.

Collapses the duplicated pipeline glue across the four _render_* functions behind three seams. Pure refactor — behaviour- and pixel-preserving.

Seam 1 — datashader orchestration (highest leverage)

Extracted _shade_datashader_aggregate() into _datashader.py: the core every datashader caller repeats — _ds_aggregate_apply_ds_norm → transparent-NA guard → _build_color_key → categorical/continuous shade dispatch. _render_shapes (previously inline) and _datashader_points now both call it, keeping only their element-specific prep (geometry transform vs point parse + spread_px) and their own outline / image / colorbar steps.

This is the seam with real maintenance history: #367/#687 (double-alpha), #372/#376 (clip/cmap-alpha), #617 (_apply_user_alpha) each had to be applied to both copies. Now there is one.

Note: since the issue was filed, points was already extracted into _datashader_points (via #714), so this seam unifies shapes-inline ↔ _datashader_points rather than two inline copies.

Seam 2 — shared preamble

Extracted _warn_groups() and _maybe_apply_transfunc(), now used by shapes, points and labels. The filter_by_coordinate_system / groups-filter blocks the audit mentioned are not actually identical across the three renderers (GeoDataFrame vs points+adata vs label-id masking), so those are intentionally left in place rather than forced into a false abstraction.

Seam 3 — labels legend path

_render_labels re-inlined _add_legend_and_colorbar's body and had drifted on na_in_legend and the categorical colorbar term. Generalized the helper with is_continuous_override / na_in_legend_override and routed labels through it, deleting the ~50-line copy. The overrides reproduce labels' exact current behaviour.

Verification

  • Non-visual suite: 415 passed, 1 skipped.
  • Visual test_plot_* (shapes/points/labels) fail identically with and without this change locally — same 217/131 split, byte-identical failing sets — confirming the diff doesn't move any rendered output. (Local visual failures are pre-existing matplotlib-vs-CI-baseline drift, unrelated to this PR.)
  • ruff check / ruff format clean.

No public API, default behaviour, or I/O changes. No new dependencies.

The four _render_* functions re-implemented the same pipeline glue. This
removes the duplication behind three seams, all behaviour/pixel-preserving
(local visual baselines fail identically with and without this change).

Seam 1 - datashader orchestration:
  Extract _shade_datashader_aggregate() into _datashader.py: the shared
  aggregate -> norm -> transparent-NA guard -> color_key -> shade dispatch.
  _render_shapes (was inline) and _datashader_points now both call it,
  keeping only their element-specific prep (geometry transform vs point
  parse + spread_px). Past datashader fixes (#367/#687, #372/#376, #617)
  each had to be applied to both copies; now there is one.

Seam 2 - shared preamble:
  Extract _warn_groups() and _maybe_apply_transfunc(), used by shapes,
  points and labels. (The cs-filter / groups-filter blocks the audit
  mentioned are not actually identical across renderers, so left as-is.)

Seam 3 - labels legend path:
  Labels re-inlined _add_legend_and_colorbar's body and had drifted on
  na_in_legend and the categorical term. Generalize the helper with
  is_continuous_override / na_in_legend_override and route labels through
  it, deleting the ~50-line copy.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.13%. Comparing base (4ba13a3) to head (2761d84).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #717   +/-   ##
=======================================
  Coverage   77.13%   77.13%           
=======================================
  Files          14       14           
  Lines        4457     4440   -17     
  Branches     1023     1014    -9     
=======================================
- Hits         3438     3425   -13     
+ Misses        661      658    -3     
+ Partials      358      357    -1     
Files with missing lines Coverage Δ
src/spatialdata_plot/pl/_datashader.py 91.28% <100.00%> (+0.77%) ⬆️
src/spatialdata_plot/pl/render.py 88.37% <100.00%> (+0.21%) ⬆️

... 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.

@timtreis timtreis changed the title refactor: collapse duplicated orchestration across the _render_* quartet (#698) refactor: collapse duplicated orchestration across the _render_* quartet Jun 14, 2026
@timtreis timtreis merged commit e0098dd into main Jun 14, 2026
7 of 8 checks passed
@timtreis timtreis deleted the refactor/issue-698-render-quartet branch June 14, 2026 20:35
Sign up for free to 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.

Refactor: collapse duplicated orchestration across the _render_* quartet (datashader block, preamble, labels legend path)

2 participants