Skip to content

Commit 8da18d2

Browse files
timtreisclaude
andcommitted
Address review: deduplicate cmap(0) call, add error tests, trim warning
- Reuse pre-computed zero_colors array instead of calling cmap(0) twice - Add tests for norm list length mismatch and empty norm list - Remove unconditional multi-cmap warning (signal-based blend handles it) - Clarify norms field naming with inline comment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 026a28d commit 8da18d2

3 files changed

Lines changed: 23 additions & 8 deletions

File tree

src/spatialdata_plot/pl/render.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,10 +1203,9 @@ def _additive_blend(
12031203
zero_colors = np.array([cm(0.0)[:3] for cm in channel_cmaps])
12041204
canvas = np.mean(zero_colors, axis=0)
12051205
composite = np.full((height, width, 3), canvas, dtype=float)
1206-
for ch, cmap in zip(channels, channel_cmaps, strict=True):
1207-
zero_rgb = np.array(cmap(0.0)[:3])
1206+
for idx, (ch, cmap) in enumerate(zip(channels, channel_cmaps, strict=True)):
12081207
rgba = cmap(np.asarray(layers[ch]))
1209-
composite += rgba[..., :3] - zero_rgb
1208+
composite += rgba[..., :3] - zero_colors[idx]
12101209
return np.clip(composite, 0, 1, out=composite)
12111210

12121211

@@ -1260,10 +1259,6 @@ def _render_images(
12601259

12611260
# True if user gave n cmaps for n channels
12621261
got_multiple_cmaps = isinstance(render_params.cmap_params, list)
1263-
if got_multiple_cmaps:
1264-
logger.warning(
1265-
"You're blending multiple cmaps. Consider using 'palette' for black-to-color compositing instead."
1266-
)
12671262

12681263
# not using got_multiple_cmaps here because of ruff :(
12691264
if isinstance(render_params.cmap_params, list) and len(render_params.cmap_params) != n_channels:

src/spatialdata_plot/pl/render_params.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ class ImageRenderParams:
264264
element: str
265265
channel: list[str] | list[int] | int | str | None = None
266266
palette: ListedColormap | list[str] | None = None
267-
norms: list[Normalize] | None = None
267+
norms: list[Normalize] | None = None # plural to distinguish from the scalar norm in CmapParams
268268
alpha: float = 1.0
269269
scale: str | None = None
270270
zorder: int = 0

tests/pl/test_render_images.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import matplotlib
33
import matplotlib.pyplot as plt
44
import numpy as np
5+
import pytest
56
import scanpy as sc
67
from matplotlib.colors import Normalize
78
from spatial_image import to_spatial_image
@@ -171,6 +172,25 @@ def test_plot_can_normalize_single_channel_via_list(self, sdata_blobs: SpatialDa
171172
cmap="Greys",
172173
).pl.show()
173174

175+
# --- Validation errors ---
176+
177+
def test_norm_list_length_mismatch_raises(self, sdata_blobs: SpatialData):
178+
"""norm list length must match number of channels."""
179+
with pytest.raises(ValueError, match="must match the number of channels"):
180+
sdata_blobs.pl.render_images(
181+
element="blobs_image",
182+
channel=[0, 1, 2],
183+
norm=[Normalize(), Normalize()], # 2 norms for 3 channels
184+
).pl.show()
185+
186+
def test_norm_list_empty_raises(self, sdata_blobs: SpatialData):
187+
"""Empty norm list is rejected at validation time."""
188+
with pytest.raises(ValueError, match="must not be empty"):
189+
sdata_blobs.pl.render_images(
190+
element="blobs_image",
191+
norm=[],
192+
).pl.show()
193+
174194
def test_plot_correctly_normalizes_multichannel_images(self, sdata_raccoon: SpatialData):
175195
sdata_raccoon["raccoon_int16"] = Image2DModel.parse(
176196
sdata_raccoon["raccoon"].data.astype(np.uint16) * 257, # 255 * 257 = 65535,

0 commit comments

Comments
 (0)