Skip to content

Commit baf57e2

Browse files
committed
fix(color): align_to_length must branch on is_categorical, not source-not-None (#700)
The pad branch keyed on `source_vector is not None`, true for BOTH categorical and the none state — but only a Categorical has `.categories`. A `none`-state outline column (all-NaN) that under-annotates the element crashed with AttributeError. Branch on is_categorical; pad the none state's na-array source with na entries. Adds the missing none-state align test. Also trims the ColorSpec/norm/_warn_groups comments per review (lean, no behaviour change).
1 parent 2490e71 commit baf57e2

3 files changed

Lines changed: 34 additions & 25 deletions

File tree

src/spatialdata_plot/pl/_color.py

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,7 @@ def _resolve_continuous_norm(values: Any, cmap_params: CmapParams) -> Normalize:
132132
if vmax is None:
133133
vmax = data_max
134134
if vmin == vmax and not isinstance(base, LogNorm):
135-
# A constant/degenerate range collapses the colormap onto its floor; fall back to the
136-
# unit interval so the single value maps to a stable color. LogNorm is exempt: 0 is
137-
# out of its domain.
135+
# degenerate range collapses the cmap onto its floor; fall back to [0, 1]. LogNorm exempt (0 not in domain).
138136
vmin, vmax = 0.0, 1.0
139137
resolved = copy(base)
140138
resolved.vmin, resolved.vmax = vmin, vmax
@@ -701,20 +699,19 @@ def _set_color_source_vec(
701699

702700
@dataclass(frozen=True, eq=False)
703701
class ColorSpec:
704-
"""Resolved color for one element layer: the explicit color-state the renderers consume.
702+
"""Resolved color for one element layer, with the color-state made explicit.
705703
706-
``colortype`` names the three states the renderers used to infer implicitly from
707-
``source_vector``/``categorical``: ``categorical`` (``source_vector`` is the Categorical),
708-
``continuous`` (``source_vector`` is None, ``color_vector`` is numeric), ``none`` (no/uncolorable
709-
value -> ``color_vector`` is the na_color, ``source_vector`` is a non-None na array).
704+
``colortype``: ``categorical`` (``source_vector`` is the Categorical), ``continuous``
705+
(``source_vector`` is None, ``color_vector`` numeric), ``none`` (uncolorable -> ``color_vector`` is
706+
na_color, ``source_vector`` a non-None na array). Trap: ``source_vector is not None`` means
707+
categorical OR none, not categorical — branch on :attr:`is_categorical`.
710708
"""
711709

712710
colortype: ColorType
713711
source_vector: ArrayLike | pd.Series | None
714712
color_vector: ArrayLike
715713

716-
# Predicates on the invariant ``colortype`` — safe to read anywhere (unlike the vectors, which
717-
# the renderers mutate after resolution). They replace scattered, typo-prone ``colortype == "..."``.
714+
# Predicates on the invariant ``colortype`` (the vectors get mutated after resolution; the type does not).
718715
@property
719716
def is_categorical(self) -> bool:
720717
return self.colortype == "categorical"
@@ -727,8 +724,7 @@ def is_continuous(self) -> bool:
727724
def is_none(self) -> bool:
728725
return self.colortype == "none"
729726

730-
# Immutable transforms — return a new spec so the renderers can thread one ``color_spec`` through
731-
# the post-resolution mutations instead of reassigning two vectors in lockstep (the sync footgun).
727+
# Immutable transforms: return a new spec so renderers thread one ``color_spec``, not two vectors in lockstep.
732728
def filter(self, mask: Any) -> ColorSpec:
733729
"""Row-subset both vectors by a boolean/index ``mask``; ``colortype`` is unchanged.
734730
@@ -753,25 +749,27 @@ def apply_transfunc(self, transfunc: Any) -> ColorSpec:
753749
def align_to_length(self, n: int, na_color: Color) -> ColorSpec:
754750
"""Pad or truncate both vectors to ``n`` rows (cross-table / rasterize outline alignment).
755751
756-
Padded rows render as ``na_color``: a categorical (hex) ``color_vector`` pads with the na_color
757-
hex — NaN would crash ``to_rgba_array``and ``source_vector`` with an absent category; a
758-
continuous vector pads with NaN so the cmap maps it to na_color.
752+
Padded rows render as ``na_color``: continuous pads with NaN; categorical/none pad the hex
753+
``color_vector`` with the na_color hex (NaN would crash ``to_rgba_array``) and the
754+
``source_vector`` with an absent category / na entry.
759755
"""
760756
vec = self.color_vector
761757
if vec is None or len(vec) == n:
762758
return self
763759
if len(vec) > n:
764760
source = None if self.source_vector is None else self.source_vector[:n]
765-
return replace(self, source_vector=source, color_vector=np.asarray(vec)[:n])
761+
return replace(self, source_vector=source, color_vector=vec[:n])
766762
pad = n - len(vec)
767-
if self.source_vector is not None:
768-
padded = np.concatenate(
769-
[np.asarray(vec, dtype=object), np.full(pad, na_color.get_hex_with_alpha(), dtype=object)]
770-
)
763+
if self.is_continuous:
764+
padded = np.concatenate([np.asarray(vec, dtype=float), np.full(pad, np.nan)])
765+
return replace(self, color_vector=padded)
766+
na_hex = na_color.get_hex_with_alpha()
767+
padded = np.concatenate([np.asarray(vec, dtype=object), np.full(pad, na_hex, dtype=object)])
768+
if self.is_categorical:
771769
source = pd.Categorical(list(self.source_vector) + [None] * pad, categories=self.source_vector.categories)
772-
return replace(self, source_vector=source, color_vector=padded)
773-
padded = np.concatenate([np.asarray(vec, dtype=float), np.full(pad, np.nan)])
774-
return replace(self, color_vector=padded)
770+
else: # none: source is an na array, not a Categorical — extend it with na entries
771+
source = np.concatenate([np.asarray(self.source_vector, dtype=object), np.full(pad, na_hex, dtype=object)])
772+
return replace(self, source_vector=source, color_vector=padded)
775773

776774

777775
def resolve_color(*args: Any, **kwargs: Any) -> ColorSpec:

src/spatialdata_plot/pl/render.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,7 @@ def _warn_groups(
269269
) -> None:
270270
"""Emit the two groups-related warnings every ``_render_*`` preamble shares."""
271271
_warn_groups_ignored_continuous(groups, colortype, col_for_color)
272-
# Only a categorical source has `.categories` to check groups against; the none state
273-
# (na-array source) must not reach _warn_missing_groups (it has no categories).
272+
# only a categorical source has `.categories`; the none state (na-array source) must not reach it
274273
if groups is not None and colortype == "categorical":
275274
_warn_missing_groups(groups, color_source_vector, col_for_color)
276275

tests/pl/test_utils.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,6 +1027,18 @@ def test_align_to_length_pads_categorical_with_na_color_hex(self):
10271027
assert list(out.color_vector[-2:]) == [na.get_hex_with_alpha()] * 2
10281028
assert pd.isna(out.source_vector[-2:]).all() # padded rows carry no category
10291029

1030+
def test_align_to_length_pads_none_state_with_na(self):
1031+
# the trap: a `none` spec has a non-None na *array* source (not a Categorical); padding it
1032+
# must not take the categorical `.categories` branch
1033+
from spatialdata_plot.pl._color import ColorSpec
1034+
1035+
na = Color("#abcdefff")
1036+
na_hex = na.get_hex_with_alpha()
1037+
spec = ColorSpec("none", np.array([na_hex, na_hex], dtype=object), np.array([na_hex, na_hex], dtype=object))
1038+
out = spec.align_to_length(4, na)
1039+
assert len(out.color_vector) == len(out.source_vector) == 4
1040+
assert list(out.color_vector) == [na_hex] * 4
1041+
10301042
def test_align_to_length_pads_continuous_with_nan(self):
10311043
out = self._cont_spec().align_to_length(6, Color())
10321044
assert out.source_vector is None

0 commit comments

Comments
 (0)