refactor: collapse duplicated orchestration across the _render_* quartet#717
Merged
Conversation
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 Report✅ All modified and coverable lines are covered by tests. 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
🚀 New features to boost your workflow:
|
This was referenced Jun 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_pointsnow 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.Seam 2 — shared preamble
Extracted
_warn_groups()and_maybe_apply_transfunc(), now used by shapes, points and labels. Thefilter_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_labelsre-inlined_add_legend_and_colorbar's body and had drifted onna_in_legendand thecategoricalcolorbar term. Generalized the helper withis_continuous_override/na_in_legend_overrideand routed labels through it, deleting the ~50-line copy. The overrides reproduce labels' exact current behaviour.Verification
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 formatclean.No public API, default behaviour, or I/O changes. No new dependencies.