Skip to content

Commit b9373c6

Browse files
committed
refactor(render): single continuous-norm feeds pixels and colorbar (#699)
PR2 of the #699 color/cmap/norm unification. Consolidates the duplicated "resolve vmin/vmax then map" logic and makes the colorbar derive from the SAME norm as the pixels, so the two are guaranteed to agree instead of being two independent computations that happened to match. Root cause of the latent mismatch: shapes fill is a facecolor-baked PatchCollection and labels are pre-baked RGB, so matplotlib never maps their norm — the colorbar range was derived by a separate path (set_clim / _append_outline_colorbar / the labels imshow norm relying on _map_color_seg autoscaling the shared norm in place). - Add `_resolve_continuous_norm(values, cmap_params)`: one pure resolver (honor explicit vmin/vmax, else finite data range, else [0, 1]). Every pixel-baking site and its matching colorbar site calls it with the SAME value vector, so they cannot diverge. - Fold the 4 duplicated inline norm blocks (`_color_vector_to_rgba` x2, `_get_collection_shape` x2) into it; drop the now-dead `norm` param of `_get_collection_shape`. - Labels: `_map_color_seg` resolves the norm instead of mutating the shared one; the labels imshow is fed the same resolved norm so its colorbar matches without the in-place-autoscale side effect. - Datashader, image, points and graph paths are unchanged (set_array / reduction bounds — they already share their norm). Behavior-preserving for normal renders: both old paths already reduced the same vector to nanmin/nanmax. The resolver adds no vmin==vmax expansion of its own (matplotlib already expands a degenerate clim by ±0.5 downstream, so the bar is unchanged; degenerate pixels stay cmap(0) as before). The only edge that unifies is an all-identical-value outline column, which previously used a [0, 1] guard — an extreme corner now consistent with the fill path. Adds unit tests for `_resolve_continuous_norm` and a non-visual integration test asserting the fill colorbar clim equals the resolved pixel range.
1 parent 15fc496 commit b9373c6

4 files changed

Lines changed: 139 additions & 79 deletions

File tree

src/spatialdata_plot/pl/render.py

Lines changed: 15 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@
7070
_get_linear_colormap,
7171
_hex_no_alpha,
7272
_join_table_for_element,
73-
_make_continuous_mappable,
7473
_map_color_seg,
7574
_maybe_set_colors,
7675
_mpl_ax_contains_elements,
@@ -80,6 +79,7 @@
8079
_prepare_transformation,
8180
_rasterize_if_necessary,
8281
_rasterize_if_necessary_datashader,
82+
_resolve_continuous_norm,
8383
_set_color_source_vec,
8484
_stream_label_centroid_stats,
8585
_validate_polygons,
@@ -510,21 +510,17 @@ def _append_outline_colorbar(
510510
) -> None:
511511
"""Append a `ColorbarSpec` for a continuous outline column.
512512
513-
No-op when ``outline_color_vector`` has no finite values. Honors user-supplied
514-
`vmin`/`vmax` on ``cmap_params.norm``; falls back to data range. Mirrors the
515-
`vmin == vmax` ±0.5 expansion used by the fill colorbar.
513+
No-op when ``outline_color_vector`` has no finite values. Derives the bar from the same
514+
resolved norm the outline pixels use, so the two agree (#699).
516515
"""
517516
arr = pd.to_numeric(pd.Series(np.asarray(outline_color_vector)), errors="coerce").to_numpy()
518-
finite = np.isfinite(arr)
519-
if not finite.any():
517+
if not np.isfinite(arr).any():
520518
return
521-
norm = cmap_params.norm
522-
vmin = norm.vmin if norm.vmin is not None else float(np.nanmin(arr[finite]))
523-
vmax = norm.vmax if norm.vmax is not None else float(np.nanmax(arr[finite]))
519+
used_norm = _resolve_continuous_norm(outline_color_vector, cmap_params)
524520
colorbar_requests.append(
525521
ColorbarSpec(
526522
ax=ax,
527-
mappable=_make_continuous_mappable(vmin, vmax, cmap_params.cmap),
523+
mappable=ScalarMappable(norm=used_norm, cmap=cmap_params.cmap),
528524
params=colorbar_params,
529525
label=outline_col,
530526
alpha=alpha,
@@ -963,7 +959,6 @@ def _render_shapes(
963959
render_params=render_params,
964960
rasterized=sc_settings._vector_friendly,
965961
cmap=None,
966-
norm=None,
967962
fill_alpha=0.0,
968963
outline_alpha=render_params.outline_alpha[0],
969964
outline_color=outline_rgba,
@@ -982,7 +977,6 @@ def _render_shapes(
982977
render_params=render_params,
983978
rasterized=sc_settings._vector_friendly,
984979
cmap=None,
985-
norm=None,
986980
fill_alpha=0.0,
987981
outline_alpha=render_params.outline_alpha[0],
988982
outline_color=render_params.outline_params.outer_outline_color.get_hex(),
@@ -1003,7 +997,6 @@ def _render_shapes(
1003997
render_params=render_params,
1004998
rasterized=sc_settings._vector_friendly,
1005999
cmap=None,
1006-
norm=None,
10071000
fill_alpha=0.0,
10081001
outline_alpha=render_params.outline_alpha[1],
10091002
outline_color=render_params.outline_params.inner_outline_color.get_hex(),
@@ -1025,7 +1018,6 @@ def _render_shapes(
10251018
render_params=render_params,
10261019
rasterized=sc_settings._vector_friendly,
10271020
cmap=render_params.cmap_params.cmap,
1028-
norm=norm,
10291021
fill_alpha=render_params.fill_alpha,
10301022
outline_alpha=0.0,
10311023
zorder=render_params.zorder,
@@ -1038,25 +1030,11 @@ def _render_shapes(
10381030
path.vertices = trans.transform(path.vertices)
10391031

10401032
if not values_are_categorical:
1041-
# Respect explicit vmin/vmax; otherwise derive from finite numeric values, falling back to [0, 1] if unavailable
1042-
vmin = render_params.cmap_params.norm.vmin
1043-
vmax = render_params.cmap_params.norm.vmax
1044-
if vmin is None or vmax is None:
1045-
numeric_values = pd.to_numeric(np.asarray(color_vector), errors="coerce")
1046-
finite_mask = np.isfinite(numeric_values)
1047-
if finite_mask.any():
1048-
data_min = float(np.nanmin(numeric_values[finite_mask]))
1049-
data_max = float(np.nanmax(numeric_values[finite_mask]))
1050-
if vmin is None:
1051-
vmin = data_min
1052-
if vmax is None:
1053-
vmax = data_max
1054-
else:
1055-
if vmin is None:
1056-
vmin = 0.0
1057-
if vmax is None:
1058-
vmax = 1.0
1059-
_cax.set_clim(vmin=vmin, vmax=vmax)
1033+
# Derive the colorbar range from the SAME resolved norm the fill pixels use, so the bar
1034+
# can't disagree with the rendered shapes (#699). (matplotlib path; no-op for the
1035+
# datashader image, whose colorbar is built separately from the reduction bounds.)
1036+
used_norm = _resolve_continuous_norm(color_vector, render_params.cmap_params)
1037+
_cax.set_clim(vmin=used_norm.vmin, vmax=used_norm.vmax)
10601038

10611039
_add_legend_and_colorbar(
10621040
ax=ax,
@@ -2460,11 +2438,14 @@ def _draw_labels(
24602438
outline_color_source_vector=outline_color_source_vector if seg_boundaries else None,
24612439
)
24622440

2441+
# `labels` is pre-baked RGB, so imshow ignores cmap/norm for pixels — they exist only to
2442+
# drive the colorbar. Feed it the SAME resolved norm `_map_color_seg` used for the pixels
2443+
# so the bar matches, instead of relying on an in-place autoscale of the shared norm (#699).
24632444
cax = ax.imshow(
24642445
labels,
24652446
rasterized=True,
24662447
cmap=None if categorical else render_params.cmap_params.cmap,
2467-
norm=None if categorical else render_params.cmap_params.norm,
2448+
norm=None if categorical else _resolve_continuous_norm(color_vector, render_params.cmap_params),
24682449
alpha=alpha,
24692450
origin="lower",
24702451
zorder=render_params.zorder,

src/spatialdata_plot/pl/utils.py

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,32 @@ def _make_continuous_mappable(vmin: float, vmax: float, cmap: Any) -> ScalarMapp
497497
return ScalarMappable(norm=Normalize(vmin=vmin, vmax=vmax), cmap=cmap)
498498

499499

500+
def _resolve_continuous_norm(values: Any, cmap_params: CmapParams) -> Normalize:
501+
"""Resolve a concrete ``Normalize`` for continuous coloring (#699).
502+
503+
Single source of truth so the rendered pixels and the colorbar derive the SAME
504+
``vmin``/``vmax`` from the SAME values — call it with the same value vector for both
505+
and they cannot disagree. Honors explicit ``cmap_params.norm`` vmin/vmax; otherwise
506+
uses the finite-value data range, falling back to ``[0, 1]`` when no finite values
507+
exist. Behavior-preserving: this reproduces the data range the existing pixel path
508+
already used (no special-casing of ``vmin == vmax``).
509+
"""
510+
base = cmap_params.norm
511+
vmin, vmax = base.vmin, base.vmax
512+
if vmin is None or vmax is None:
513+
arr = np.asarray(values)
514+
if not np.issubdtype(arr.dtype, np.number):
515+
arr = pd.to_numeric(pd.Series(arr.ravel()), errors="coerce").to_numpy()
516+
finite = np.isfinite(arr)
517+
data_min = float(np.nanmin(arr[finite])) if finite.any() else 0.0
518+
data_max = float(np.nanmax(arr[finite])) if finite.any() else 1.0
519+
if vmin is None:
520+
vmin = data_min
521+
if vmax is None:
522+
vmax = data_max
523+
return Normalize(vmin=vmin, vmax=vmax, clip=base.clip)
524+
525+
500526
def _apply_mask_to_outline_vectors(
501527
outline_color_vector: Any,
502528
outline_color_source_vector: pd.Series | None,
@@ -577,15 +603,7 @@ def _color_vector_to_rgba(
577603
if np.issubdtype(arr.dtype, np.number):
578604
finite_mask = np.isfinite(arr)
579605
if finite_mask.any():
580-
norm = cmap_params.norm
581-
if norm.vmin is None or norm.vmax is None:
582-
vmin = float(np.nanmin(arr[finite_mask]))
583-
vmax = float(np.nanmax(arr[finite_mask]))
584-
if not np.isfinite(vmin) or not np.isfinite(vmax) or vmin == vmax:
585-
vmin, vmax = 0.0, 1.0
586-
used_norm = Normalize(vmin=vmin, vmax=vmax, clip=False)
587-
else:
588-
used_norm = norm
606+
used_norm = _resolve_continuous_norm(arr, cmap_params)
589607
rgba[finite_mask] = cmap_params.cmap(used_norm(arr[finite_mask]))
590608
return rgba
591609

@@ -594,15 +612,7 @@ def _color_vector_to_rgba(
594612
num = pd.to_numeric(series, errors="coerce").to_numpy()
595613
is_num = np.isfinite(num)
596614
if is_num.any():
597-
norm = cmap_params.norm
598-
if norm.vmin is None or norm.vmax is None:
599-
vmin = float(np.nanmin(num[is_num]))
600-
vmax = float(np.nanmax(num[is_num]))
601-
if not np.isfinite(vmin) or not np.isfinite(vmax) or vmin == vmax:
602-
vmin, vmax = 0.0, 1.0
603-
used_norm = Normalize(vmin=vmin, vmax=vmax, clip=False)
604-
else:
605-
used_norm = norm
615+
used_norm = _resolve_continuous_norm(num, cmap_params)
606616
rgba[is_num] = cmap_params.cmap(used_norm(num[is_num]))
607617
color_mask = (~is_num) & series.notna().to_numpy()
608618
if color_mask.any():
@@ -697,7 +707,6 @@ def _get_collection_shape(
697707
shapes: list[GeoDataFrame],
698708
c: Any,
699709
s: float,
700-
norm: Any,
701710
render_params: ShapesRenderParams,
702711
fill_alpha: None | float = None,
703712
outline_alpha: None | float = None,
@@ -745,23 +754,12 @@ def _as_rgba_array(x: Any) -> np.ndarray:
745754
elif c_arr.ndim == 1 and len(c_arr) == len(shapes) and np.issubdtype(c_arr.dtype, np.number):
746755
finite_mask = np.isfinite(c_arr)
747756

748-
# Select or build a normalization that ignores NaNs for scaling
749-
if isinstance(norm, Normalize):
750-
used_norm: Normalize = norm
751-
else:
752-
if finite_mask.any():
753-
vmin = float(np.nanmin(c_arr[finite_mask]))
754-
vmax = float(np.nanmax(c_arr[finite_mask]))
755-
if not np.isfinite(vmin) or not np.isfinite(vmax) or vmin == vmax:
756-
vmin, vmax = 0.0, 1.0
757-
else:
758-
vmin, vmax = 0.0, 1.0
759-
used_norm = colors.Normalize(vmin=vmin, vmax=vmax, clip=False)
760-
761-
# Map finite values through cmap(norm(.)); NaNs get na_color
757+
# Map finite values through cmap(norm(.)); NaNs get na_color. The norm is resolved
758+
# from the same values the colorbar uses, so pixels and colorbar agree (#699).
762759
fill_c = np.empty((len(c_arr), 4), dtype=float)
763760
fill_c[:] = na_rgba
764761
if finite_mask.any():
762+
used_norm = _resolve_continuous_norm(c_arr, render_params.cmap_params)
765763
fill_c[finite_mask] = cmap(used_norm(c_arr[finite_mask]))
766764

767765
elif c_arr.ndim == 1 and len(c_arr) == len(shapes) and c_arr.dtype == object:
@@ -776,14 +774,7 @@ def _as_rgba_array(x: Any) -> np.ndarray:
776774

777775
# numeric entries via cmap(norm)
778776
if is_num.any():
779-
if isinstance(norm, Normalize):
780-
used_norm = norm
781-
else:
782-
vmin = float(np.nanmin(num[is_num])) if is_num.any() else 0.0
783-
vmax = float(np.nanmax(num[is_num])) if is_num.any() else 1.0
784-
if not np.isfinite(vmin) or not np.isfinite(vmax) or vmin == vmax:
785-
vmin, vmax = 0.0, 1.0
786-
used_norm = colors.Normalize(vmin=vmin, vmax=vmax, clip=False)
777+
used_norm = _resolve_continuous_norm(num, render_params.cmap_params)
787778
fill_c[is_num] = cmap(used_norm(num[is_num]))
788779

789780
# non-numeric, non-NaN entries as explicit colors
@@ -1531,9 +1522,12 @@ def _map_color_seg(
15311522
# Case B: user wants to plot a continous column
15321523
if isinstance(color_vector, pd.Series):
15331524
color_vector = color_vector.to_numpy()
1534-
# normalize only the not nan values, else the whole array would contain only nan values
1525+
# normalize only the not nan values, else the whole array would contain only nan values.
1526+
# Resolve the norm from the same values the colorbar uses so the two agree without relying
1527+
# on a shared-norm autoscale side effect (#699).
15351528
normed_color_vector = color_vector.copy().astype(float)
1536-
normed_color_vector[~np.isnan(normed_color_vector)] = cmap_params.norm(
1529+
used_norm = _resolve_continuous_norm(normed_color_vector, cmap_params)
1530+
normed_color_vector[~np.isnan(normed_color_vector)] = used_norm(
15371531
normed_color_vector[~np.isnan(normed_color_vector)]
15381532
)
15391533
cols = cmap_params.cmap(normed_color_vector)
@@ -1558,7 +1552,8 @@ def _map_color_seg(
15581552
assert all(_is_color_like(c) for c in color_vector), "Not all values are color-like."
15591553
cols = colors.to_rgba_array(color_vector)
15601554
else:
1561-
cols = cmap_params.cmap(cmap_params.norm(color_vector))
1555+
used_norm = _resolve_continuous_norm(color_vector, cmap_params)
1556+
cols = cmap_params.cmap(used_norm(color_vector))
15621557

15631558
if seg_erosionpx is not None:
15641559
val_im[val_im == erosion(val_im, footprint_rectangle((seg_erosionpx, seg_erosionpx)))] = 0
@@ -1592,7 +1587,7 @@ def _map_color_seg(
15921587
normed = ov.copy().astype(float)
15931588
finite = ~np.isnan(normed)
15941589
if finite.any():
1595-
normed[finite] = cmap_params.norm(normed[finite])
1590+
normed[finite] = _resolve_continuous_norm(ov, cmap_params)(normed[finite])
15961591
outline_cols = cmap_params.cmap(normed)
15971592
outline_val_im = map_array(seg, cell_id, cell_id)
15981593
if seg_erosionpx is not None:

tests/pl/test_render_shapes.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1780,3 +1780,33 @@ def test_render_shapes_as_points_default_is_matplotlib(sdata_blobs: SpatialData)
17801780
sdata_blobs.pl.render_shapes("blobs_circles", as_points=True, size=50).pl.show(ax=ax)
17811781
assert len(ax.collections) == 1 and len(ax.images) == 0
17821782
plt.close(fig)
1783+
1784+
1785+
def _shapes_sdata_with_values(values: list[float]) -> SpatialData:
1786+
"""A shapes element + annotating table carrying a continuous column ``val``."""
1787+
n = len(values)
1788+
polys = [Polygon([(i, 0.0), (i, 1.0), (i + 1, 1.0), (i + 1, 0.0)]) for i in range(n)]
1789+
geo = gpd.GeoDataFrame(geometry=gpd.GeoSeries(polys))
1790+
sdata = SpatialData(shapes={"p": ShapesModel.parse(geo)})
1791+
adata = AnnData(pd.DataFrame({"p": [str(i) for i in range(n)]}))
1792+
adata.obs["region"] = "p"
1793+
adata.obs["instance_id"] = list(range(n))
1794+
adata.obs["val"] = list(values)
1795+
sdata["table"] = TableModel.parse(adata, region="p", region_key="region", instance_key="instance_id")
1796+
return sdata
1797+
1798+
1799+
def _fill_collection_clim(values: list[float]) -> tuple[float, float]:
1800+
from matplotlib.collections import PatchCollection
1801+
1802+
fig, ax = plt.subplots()
1803+
_shapes_sdata_with_values(values).pl.render_shapes(color="val").pl.show(ax=ax)
1804+
clims = [c.get_clim() for c in ax.collections if isinstance(c, PatchCollection)]
1805+
plt.close(fig)
1806+
assert len(clims) == 1
1807+
return clims[0]
1808+
1809+
1810+
def test_continuous_fill_colorbar_matches_pixel_range():
1811+
"""#699: the fill colorbar clim is the resolved data range, so the bar matches the shapes."""
1812+
assert _fill_collection_clim([0.0, 1.0, 2.0, 3.0]) == (0.0, 3.0)

tests/pl/test_utils.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -851,3 +851,57 @@ def test_cmap_with_alpha_bad_color_uses_na_with_requested_alpha(self):
851851
np.testing.assert_allclose(bad[:3], to_rgba(na.get_hex_with_alpha())[:3], atol=1e-6)
852852
# historical `_lut[:, -1] = alpha` overwrote the bad-row alpha too
853853
assert bad[3] == 0.25
854+
855+
856+
class TestResolveContinuousNorm:
857+
"""Unit tests for `_resolve_continuous_norm` — the single norm source feeding pixels + colorbar (#699)."""
858+
859+
@staticmethod
860+
def _params(vmin: float | None = None, vmax: float | None = None) -> CmapParams:
861+
from matplotlib import colormaps
862+
from matplotlib.colors import Normalize
863+
864+
return CmapParams(cmap=colormaps["viridis"], norm=Normalize(vmin=vmin, vmax=vmax, clip=False), na_color=Color())
865+
866+
def test_honors_explicit_vmin_vmax(self):
867+
from spatialdata_plot.pl.utils import _resolve_continuous_norm
868+
869+
norm = _resolve_continuous_norm(np.array([0.0, 100.0]), self._params(vmin=10.0, vmax=20.0))
870+
assert (norm.vmin, norm.vmax) == (10.0, 20.0)
871+
872+
def test_derives_data_range_ignoring_nan_and_inf(self):
873+
from spatialdata_plot.pl.utils import _resolve_continuous_norm
874+
875+
norm = _resolve_continuous_norm(np.array([1.0, np.nan, 5.0, np.inf, -np.inf]), self._params())
876+
assert (norm.vmin, norm.vmax) == (1.0, 5.0)
877+
878+
def test_all_nan_falls_back_to_unit_range(self):
879+
from spatialdata_plot.pl.utils import _resolve_continuous_norm
880+
881+
norm = _resolve_continuous_norm(np.array([np.nan, np.nan]), self._params())
882+
assert (norm.vmin, norm.vmax) == (0.0, 1.0)
883+
884+
def test_degenerate_range_is_not_expanded(self):
885+
# behavior-preserving: a single distinct value stays degenerate (no invented +/-0.5)
886+
from spatialdata_plot.pl.utils import _resolve_continuous_norm
887+
888+
norm = _resolve_continuous_norm(np.array([5.0, 5.0, 5.0]), self._params())
889+
assert (norm.vmin, norm.vmax) == (5.0, 5.0)
890+
891+
def test_object_dtype_coerces_color_strings_to_nan(self):
892+
from spatialdata_plot.pl.utils import _resolve_continuous_norm
893+
894+
norm = _resolve_continuous_norm(np.array([1.0, "red", 9.0], dtype=object), self._params())
895+
assert (norm.vmin, norm.vmax) == (1.0, 9.0)
896+
897+
def test_same_values_give_identical_norm_and_never_mutate_shared(self):
898+
# the core #699 invariant: pixels and colorbar call this with the same vector -> same result,
899+
# and the shared CmapParams.norm is never autoscaled in place.
900+
from spatialdata_plot.pl.utils import _resolve_continuous_norm
901+
902+
params = self._params()
903+
vals = np.array([2.0, 7.0, np.nan, 4.0])
904+
a = _resolve_continuous_norm(vals, params)
905+
b = _resolve_continuous_norm(vals, params)
906+
assert (a.vmin, a.vmax) == (b.vmin, b.vmax) == (2.0, 7.0)
907+
assert params.norm.vmin is None and params.norm.vmax is None

0 commit comments

Comments
 (0)