Skip to content

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

Description

@timtreis

Summary

The four _render_* functions in src/spatialdata_plot/pl/render.py independently re-implement the same pipeline glue. Three concrete seams are copy-pasted-with-variation and worth collapsing — most importantly the datashader orchestration block, which every past datashader fix had to be applied to twice.

Seam 1 — datashader orchestration block (highest leverage)

_render_shapes (render.py:769-905) vs _render_points (render.py:1243-1379): ~137 lines each, and the middle ~65 lines are line-for-line identical — build canvas → ensure color column → color_by_categorical_ds_aggregate_apply_ds_norm → na/transparent-NaN guard → _build_color_key → shade dispatch → _render_ds_image_build_ds_colorbar. The low-level helpers already live in _datashader.py; only the glue duplicates.

The maintenance tax is real and historical: #617 (_apply_user_alpha), #367/#687 (double-alpha), #372/#376 (clip/cmap-alpha) each required parallel edits to both copies.

Fix: extract render_datashader_aggregate(...) -> ScalarMappable | None into _datashader.py; both callers keep only their element-specific prep (geometry transform vs PointsModel.parse + spread_px). ~270 → ~140 lines.

Seam 2 — shared render preamble

Every _render_* opens with the same movement: _log_context.set_check_obs_var_shadowfilter_by_coordinate_system(filter_tables=False) → table join → _set_color_source_vec → groups/NaN/transfunc filtering. The groups guard + _filter_groups_transparent_na + transfunc application is byte-identical across shapes (render.py:687-703), points (1212-1227), and labels (2198-2222).

Fix: extract _filter_cs, _warn_and_filter_groups, _maybe_apply_transfunc.

Seam 3 — labels bypasses _add_legend_and_colorbar

Shapes and points finish via _add_legend_and_colorbar (render.py:328). Labels (render.py:2309-2362) re-inlines that helper's body instead of calling it, and has already drifted on na_in_legend and the categorical term — exactly the kind of silent per-renderer divergence that causes bugs in one renderer only.

Fix: generalize _add_legend_and_colorbar to accept the two varying inputs (is_continuous_override, na_in_legend_override) and route labels through it; deletes ~50 lines from labels.

Risk / effort

Effort: medium-high · Risk: medium — these are the visual hot path; pixel-affecting changes need CI baseline regeneration. Do Seam 1 first (cleanest, already-extracted helpers behind it). Seams 2/3 pair naturally with the show() decomposition and the color-pipeline unification filed alongside this audit.


Part of a maintainability/refactor audit of main.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions