Skip to content

Refactor: unify color/cmap/norm pipeline (CmapParams methods, single continuous-norm, one image-composite helper) #699

Description

@timtreis

Summary

Color / cmap / norm resolution is scattered and re-implemented across render.py and utils.py, with genuine inconsistencies (not just duplication) — including a colorbar-vs-pixels mismatch and a private-matplotlib-API poke. Unifying it removes a recurring bug class and shrinks every render function.

1. CmapParams is a dumb data bag; callers re-implement its safe handling

render_params.py CmapParams carries cmap, norm, na_color, cmap_is_default but no behavior, so every consumer re-does the same operations:

  • "copy the stateful Normalize before use" is hand-written 5× (render.py:705, 1231, 1765, 1862). Forgetting it risks cross-channel/cross-call mutation.
  • cmap-alpha is applied by poking matplotlib's private _lut (render.py:1803-1804) — fragile across matplotlib versions and against the "no private APIs of other packages" guideline.
  • na_color → hex conversions and isinstance(na_color, Color) guards are scattered ~10× (utils.py:1411/1468/1898/..., render.py:630/1318).

Fix: give CmapParams methods — fresh_norm(), cmap_with_alpha(alpha), is_user_cmap, na_hex. Behavior-preserving; spot-check the 1-channel baseline for the cmap-alpha change.

2. Continuous→RGBA normalization reimplemented 4–6× with drift

The "NaN-safe normalize then cmap(norm(.))" logic exists in _map_color_seg (utils.py:1510-1514, 1567-1571), _get_collection_shape Case B (render.py:650-669) and again at 683-691. They have already drifted: _map_color_seg uses ~np.isnan, _get_collection_shape uses np.isfinite (differ on ±inf); the vmin==vmax guard exists in one but not the other. The colorbar span is derived independently from the pixel span — so the colorbar can disagree with the rendered pixels (the #687/#688 churn class).

Fix: a single resolve_continuous_norm(values, cmap_params) -> Normalize that feeds both the pixels and the colorbar ScalarMappable, plus _continuous_to_rgba(values, cmap_params).

3. The four _render_images blend branches collapse to one helper

Paths 2A/2B/2C/2D (render.py:~1877-2015) are six copies of stack → reduce → (clip) with three different reductions. Collapse to one _composite_channels(layers, channel_cmaps, alpha). (The averaging-vs-additive inconsistency in this same code is tracked separately in findings/multichannel-compositing-bugs.md; unifying the helper is the structural enabler.)

Sequencing / risk

Land the safe foundations first — CmapParams methods (#1) — then resolve_continuous_norm (#2), then the composite helper (#3). Effort: medium. Risk: low for the CmapParams methods (no baseline impact); medium for the norm/composite unification (CI baseline regen). #2 also fixes the colorbar-vs-pixels mismatch, so it's correctness, not just cleanup.


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