Skip to content

Deduplicate shared logic between _render_shapes and _render_points#551

Draft
timtreis wants to merge 6 commits intomainfrom
refactor/dedup-render-shapes-points
Draft

Deduplicate shared logic between _render_shapes and _render_points#551
timtreis wants to merge 6 commits intomainfrom
refactor/dedup-render-shapes-points

Conversation

@timtreis
Copy link
Member

Summary

  • Extracts 8 shared helper functions from the near-identical datashader rendering paths in _render_shapes() (519 lines) and _render_points() (437 lines), eliminating ~240 lines of duplicated code
  • Refactors the show() dispatch loop from 4 if/elif branches to a table-driven pattern
  • No public API changes, no behavioral changes

Motivation

_render_shapes and _render_points followed the same structure with large blocks of near-identical code (norm handling, aggregation, color mapping, NaN shading, image rendering, colorbar construction, decoration). Every bug fix or feature had to be manually replicated in both functions, and they were already drifting (different default reductions, subtly different categorical detection).

Extracted helpers

Helper What it replaces
_apply_datashader_norm Norm vmin/vmax edge-case handling (13 lines x2)
_datashader_aggregate Categorical/continuous/no-color aggregation (30 lines x2)
_datashader_shade_continuous Continuous color mapping + spread + NaN shading (40 lines x2)
_datashader_shade_categorical Categorical/no-color color mapping (10 lines x2)
_render_datashader_result RGBA image rendering + NaN overlay (10 lines x2)
_build_datashader_colorbar_mappable ScalarMappable construction (13 lines x2)
_make_palette ListedColormap construction (4 lines x2)
_decorate_render Legend/colorbar/scalebar decoration (25 lines x2)

Test plan

  • All existing tests pass (no public API changes)
  • CI baseline images match (pure internal refactor)
  • Verify shapes and points rendering is visually identical

🤖 Generated with Claude Code

@timtreis timtreis force-pushed the refactor/dedup-render-shapes-points branch 2 times, most recently from 2a85988 to 1d8ac75 Compare March 19, 2026 20:10
Extract 8 helper functions from the near-identical datashader rendering
paths in _render_shapes() and _render_points():

- _apply_datashader_norm: norm vmin/vmax edge-case handling
- _build_datashader_colorbar_mappable: ScalarMappable construction
- _datashader_aggregate: categorical/continuous/no-color aggregation
- _datashader_shade_continuous: continuous color mapping + spread + NaN
- _datashader_shade_categorical: categorical/no-color color mapping
- _render_datashader_result: RGBA image rendering + NaN overlay
- _make_palette: ListedColormap construction
- _decorate_render: legend/colorbar/scalebar decoration

Also refactor the show() dispatch loop in basic.py from 4 if/elif
branches to a table-driven pattern.

No public API changes. No behavioral changes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@timtreis timtreis force-pushed the refactor/dedup-render-shapes-points branch from 1d8ac75 to db51af4 Compare March 19, 2026 20:17
@codecov-commenter
Copy link

codecov-commenter commented Mar 19, 2026

Codecov Report

❌ Patch coverage is 93.60465% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.66%. Comparing base (2893343) to head (2d8f0da).

Files with missing lines Patch % Lines
src/spatialdata_plot/pl/_datashader.py 91.91% 4 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #551      +/-   ##
==========================================
- Coverage   73.68%   73.66%   -0.03%     
==========================================
  Files           9       10       +1     
  Lines        2744     2738       -6     
  Branches      651      637      -14     
==========================================
- Hits         2022     2017       -5     
  Misses        447      447              
+ Partials      275      274       -1     
Files with missing lines Coverage Δ
src/spatialdata_plot/pl/render.py 81.87% <100.00%> (-2.14%) ⬇️
src/spatialdata_plot/pl/_datashader.py 91.91% <91.91%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

timtreis and others added 5 commits March 19, 2026 21:30
- _render_ds_outlines: consolidates the 40-line outline aggregation +
  shading + rendering block (outer + inner) into a single loop
- _build_color_key: extracts the identical color key construction from
  both shapes and points datashader paths

The shapes and points datashader pipelines now read nearly identically,
differing only in parameter values.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Functions:
- _datashader_aggregate -> _ds_aggregate (consistent prefix)
- _datashader_shade_continuous -> _ds_shade_continuous
- _datashader_shade_categorical -> _ds_shade_categorical
- _apply_datashader_norm -> _apply_ds_norm
- _build_datashader_colorbar_mappable -> _build_ds_colorbar
- _render_datashader_result -> _render_ds_image
- _decorate_render -> _add_legend_and_colorbar

Variables:
- aggregate_with_reduction -> reduction_bounds
- continuous_nan_agg -> nan_agg
- continuous_nan_shaded -> nan_shaded
- ds_result -> shaded
- ds_span -> color_span

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All datashader-specific aggregation, shading, and rendering helpers
now live in pl/_datashader.py. render.py imports them and focuses on
the element-type-specific orchestration logic.

Moved:
- _ds_aggregate, _apply_ds_norm, _build_color_key
- _ds_shade_continuous, _ds_shade_categorical
- _render_ds_image, _render_ds_outlines, _build_ds_colorbar
- _coerce_categorical_source, _build_datashader_color_key
- _inject_ds_nan_sentinel, _DS_NAN_CATEGORY, _DsReduction

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants