Skip to content

Commit 54d66aa

Browse files
timtreisclaude
andcommitted
Simplify overengineered functions from PRs #427/#547
- _coerce_categorical_source: remove unreachable dd.Series, np.ndarray, and pd.Categorical branches (callers always pass pd.Series). 27→4 lines. - _build_datashader_color_key: replace O(n*k) flatnonzero scans with a single pass over codes. Also handles NaN sentinel via the same fallback. - _build_alignment_dtype_hint: remove overly broad exception catching (TypeError, AttributeError), redundant hasattr checks, and the color-index comparison that duplicates the element-index check. 32→12 lines. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 19e79ff commit 54d66aa

2 files changed

Lines changed: 26 additions & 63 deletions

File tree

src/spatialdata_plot/pl/render.py

Lines changed: 15 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -69,33 +69,11 @@
6969
_DS_NAN_CATEGORY = "ds_nan"
7070

7171

72-
def _coerce_categorical_source(cat_source: Any) -> pd.Categorical:
73-
"""Return a pandas Categorical from known, concrete sources only.
74-
75-
Raises
76-
------
77-
TypeError
78-
If *cat_source* is not a ``dd.Series``, ``pd.Series``,
79-
``pd.Categorical``, or ``np.ndarray``.
80-
"""
81-
if isinstance(cat_source, dd.Series):
82-
if isinstance(cat_source.dtype, pd.CategoricalDtype) and getattr(cat_source.cat, "known", True) is False:
83-
cat_source = cat_source.cat.as_known()
84-
cat_source = cat_source.compute()
85-
86-
if isinstance(cat_source, pd.Series):
87-
if isinstance(cat_source.dtype, pd.CategoricalDtype):
88-
return cat_source.array
89-
return pd.Categorical(cat_source)
90-
if isinstance(cat_source, pd.Categorical):
91-
return cat_source
92-
if isinstance(cat_source, np.ndarray):
93-
return pd.Categorical(cat_source)
94-
95-
raise TypeError(
96-
f"Cannot coerce {type(cat_source).__name__} to pd.Categorical. "
97-
"Expected dd.Series, pd.Series, pd.Categorical, or np.ndarray."
98-
)
72+
def _coerce_categorical_source(series: pd.Series) -> pd.Categorical:
73+
"""Return a ``pd.Categorical`` from a pandas Series."""
74+
if isinstance(series.dtype, pd.CategoricalDtype):
75+
return series.array
76+
return pd.Categorical(series)
9977

10078

10179
def _build_datashader_color_key(
@@ -104,18 +82,17 @@ def _build_datashader_color_key(
10482
na_color_hex: str,
10583
) -> dict[str, str]:
10684
"""Build a datashader ``color_key`` dict from a categorical series and its color vector."""
85+
na_hex = _hex_no_alpha(na_color_hex) if na_color_hex.startswith("#") else na_color_hex
86+
# Map each category to its first-occurrence color via codes
10787
colors_arr = np.asarray(color_vector, dtype=object)
108-
color_key: dict[str, str] = {}
109-
for cat in cat_series.categories:
110-
if cat == _DS_NAN_CATEGORY:
111-
key_color = na_color_hex
112-
else:
113-
idx = np.flatnonzero(cat_series == cat)
114-
key_color = colors_arr[idx[0]] if idx.size else na_color_hex
115-
if isinstance(key_color, str) and key_color.startswith("#"):
116-
key_color = _hex_no_alpha(key_color)
117-
color_key[str(cat)] = key_color
118-
return color_key
88+
first_color: dict[str, str] = {}
89+
for code, color in zip(cat_series.codes, colors_arr, strict=False):
90+
if code < 0:
91+
continue
92+
cat_name = str(cat_series.categories[code])
93+
if cat_name not in first_color:
94+
first_color[cat_name] = _hex_no_alpha(color) if isinstance(color, str) and color.startswith("#") else color
95+
return {str(c): first_color.get(str(c), na_hex) for c in cat_series.categories}
11996

12097

12198
def _inject_ds_nan_sentinel(series: pd.Series, sentinel: str = _DS_NAN_CATEGORY) -> pd.Series:

src/spatialdata_plot/pl/utils.py

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -990,31 +990,17 @@ def _build_alignment_dtype_hint(
990990
table_name: str | None,
991991
) -> str:
992992
"""Build a diagnostic hint string for dtype mismatches between element and table indices."""
993-
hints: list[str] = []
994-
color_index_dtype = getattr(color_series.index, "dtype", None)
995-
element_index_dtype = getattr(getattr(element, "index", None), "dtype", None) if element is not None else None
996-
997-
table_instance_dtype = None
998-
instance_key = None
999-
if table_name is not None and sdata is not None and table_name in sdata.tables:
1000-
table = sdata.tables[table_name]
1001-
try:
1002-
_, _, instance_key = get_table_keys(table)
1003-
except (KeyError, ValueError, TypeError, AttributeError):
1004-
instance_key = None
1005-
if instance_key is not None and hasattr(table, "obs") and instance_key in table.obs:
1006-
table_instance_dtype = table.obs[instance_key].dtype
1007-
1008-
if (
1009-
element_index_dtype is not None
1010-
and table_instance_dtype is not None
1011-
and element_index_dtype != table_instance_dtype
1012-
):
1013-
hints.append(f"element index dtype is {element_index_dtype}, '{instance_key}' dtype is {table_instance_dtype}")
1014-
if color_index_dtype is not None and element_index_dtype is not None and color_index_dtype != element_index_dtype:
1015-
hints.append(f"color index dtype is {color_index_dtype}, element index dtype is {element_index_dtype}")
1016-
1017-
return f" (hint: {'; '.join(hints)})" if hints else ""
993+
el_dtype = getattr(getattr(element, "index", None), "dtype", None)
994+
if el_dtype is None or table_name is None or sdata is None or table_name not in sdata.tables:
995+
return ""
996+
try:
997+
_, _, instance_key = get_table_keys(sdata.tables[table_name])
998+
except (KeyError, ValueError):
999+
return ""
1000+
tbl_dtype = sdata.tables[table_name].obs[instance_key].dtype
1001+
if el_dtype != tbl_dtype:
1002+
return f" (hint: element index dtype is {el_dtype}, '{instance_key}' dtype is {tbl_dtype})"
1003+
return ""
10181004

10191005

10201006
def _set_color_source_vec(

0 commit comments

Comments
 (0)