Skip to content

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

@timtreis

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.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions