Skip to content

Commit 2c31a4d

Browse files
committed
refactor(render_points): tighten codes+palette per review
- datashader strip: when no two categories collapse to the same stripped colour (the common case) reuse the int8 codes via rename_categories instead of building an n-length int64 array; only remap (at the original int width) on a real collision. - _scatter_points: guard the categorical codes path against an empty Categorical (len check) so it can't hit BoundaryNorm with zero colours; correct the NaN comment. - trim a narrating comment; dedup the scatter-kwargs in the new tests. Output unchanged: strip is byte-equivalent to the old factorize path (verified incl. alpha-collision and NaN); 63 non-visual render_points tests pass.
1 parent 9707295 commit 2c31a4d

3 files changed

Lines changed: 18 additions & 32 deletions

File tree

src/spatialdata_plot/pl/_datashader.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,7 @@ def _build_datashader_color_key(
110110
# avoiding a Python loop over all points. See #379.
111111
unique_codes, first_indices = np.unique(codes, return_index=True)
112112

113-
# Index color_vector only at those first occurrences (one per category) rather than expanding the
114-
# whole per-point vector to an object array — color_vector is a compact pd.Categorical at scale.
113+
# Index only the first occurrence per category — color_vector is a compact pd.Categorical at scale.
115114
first_color: dict[str, str] = {}
116115
for code, idx in zip(unique_codes, first_indices, strict=True):
117116
if code < 0 or idx >= len(color_vector):
@@ -404,14 +403,17 @@ def _shade_datashader_aggregate(
404403
and isinstance(color_vector[0], str)
405404
and color_vector[0].startswith("#")
406405
):
407-
# Strip alpha on the unique colours, never on the per-point vector. color_vector is already a
408-
# pd.Categorical (codes + a small palette) at scale: strip its k categories and remap the codes,
409-
# staying compact. (Plain-array fallback factorizes in O(n) — hash, no sort.)
406+
# Strip alpha on the k categories, never on the per-point vector — color_vector is a compact
407+
# pd.Categorical at scale. (Plain-array fallback factorizes in O(n): hash, no sort.)
410408
if isinstance(color_vector, pd.Categorical):
411-
stripped = np.asarray([_hex_no_alpha(c) for c in color_vector.categories])
412-
uniq_codes, uniques = pd.factorize(stripped) # dedup over k categories, not n points
413-
new_codes = np.where(color_vector.codes >= 0, uniq_codes[color_vector.codes], -1)
414-
color_vector = pd.Categorical.from_codes(new_codes, categories=uniques)
409+
stripped = [_hex_no_alpha(c) for c in color_vector.categories]
410+
uniq_codes, uniques = pd.factorize(np.asarray(stripped))
411+
if len(uniques) == len(stripped):
412+
color_vector = color_vector.rename_categories(stripped) # no collisions: reuse the codes as-is
413+
else: # two categories share a stripped colour: remap, keeping the compact int width
414+
remapped = uniq_codes[color_vector.codes].astype(color_vector.codes.dtype)
415+
remapped[color_vector.codes < 0] = -1
416+
color_vector = pd.Categorical.from_codes(remapped, categories=uniques)
415417
else:
416418
codes, uniques = pd.factorize(np.asarray(color_vector))
417419
color_vector = np.asarray([_hex_no_alpha(c) for c in uniques])[codes]

src/spatialdata_plot/pl/render.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,11 +1057,11 @@ def _scatter_points(
10571057
# machinery — the dominant cost at scale (10M points: ~9s -> ~3.7s) — for a visually identical result.
10581058
# Numeric vectors keep the ``c=``/``cmap``/``norm`` path (they need the colormap).
10591059
color_kwargs: dict[str, Any]
1060-
if isinstance(color_vector, pd.Categorical) and (color_vector.codes >= 0).all():
1060+
if isinstance(color_vector, pd.Categorical) and len(color_vector) and (color_vector.codes >= 0).all():
10611061
# Categorical hex colours: pass the int codes + a ListedColormap of the (few) category colours
10621062
# instead of expanding to a per-point hex object array (~8x more memory at scale). BoundaryNorm
10631063
# edges at the half-integers map code i -> colormap entry i exactly, so the RGBA is identical.
1064-
# (_color.py bakes NaN into the na_color category, so codes >= 0; a stray -1 falls back below.)
1064+
# _color.py bakes NaN into the na_color category, so codes >= 0 here; anything else takes the path below.
10651065
categories = list(color_vector.categories)
10661066
if len(categories) == 1:
10671067
color_kwargs = {"color": str(categories[0])} # uniform: scalar color=, skip per-point machinery

tests/pl/test_render_points.py

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1349,22 +1349,17 @@ def _categorical_hex_vector(n: int, k: int, seed: int = 0) -> pd.Categorical:
13491349
return pd.Categorical(pd.Series(src).map(palette))
13501350

13511351

1352+
_SCATTER_KW = {"size": 10.0, "cmap": "viridis", "norm": Normalize(), "alpha": 1.0, "trans_data": None, "zorder": 1}
1353+
1354+
13521355
def test_scatter_points_categorical_codes_match_per_point_hex():
13531356
# The codes+ListedColormap path must produce byte-identical RGBA to passing the per-point hex array.
13541357
cv = _categorical_hex_vector(400, k=6, seed=1)
13551358
rng = np.random.default_rng(2)
13561359
x, y = rng.random(400), rng.random(400)
1357-
common = {
1358-
"size": 10.0,
1359-
"cmap": plt.get_cmap("viridis"),
1360-
"norm": Normalize(),
1361-
"alpha": 1.0,
1362-
"trans_data": None,
1363-
"zorder": 1,
1364-
}
13651360

13661361
fig, ax = plt.subplots()
1367-
sc_codes = _scatter_points(ax, x, y, cv, **common) # exercises the new codes path
1362+
sc_codes = _scatter_points(ax, x, y, cv, **_SCATTER_KW) # exercises the new codes path
13681363
sc_codes.update_scalarmappable()
13691364
fc_codes = sc_codes.get_facecolors()
13701365
plt.close(fig)
@@ -1382,18 +1377,7 @@ def test_scatter_points_single_category_uses_scalar_color():
13821377
cv = _categorical_hex_vector(300, k=1, seed=3)
13831378
rng = np.random.default_rng(5)
13841379
fig, ax = plt.subplots()
1385-
sc = _scatter_points(
1386-
ax,
1387-
rng.random(300),
1388-
rng.random(300),
1389-
cv,
1390-
size=10.0,
1391-
cmap=plt.get_cmap("viridis"),
1392-
norm=Normalize(),
1393-
alpha=1.0,
1394-
trans_data=None,
1395-
zorder=1,
1396-
)
1380+
sc = _scatter_points(ax, rng.random(300), rng.random(300), cv, **_SCATTER_KW)
13971381
assert sc.get_facecolors().shape[0] == 1 # scalar color -> single facecolor
13981382
plt.close(fig)
13991383

0 commit comments

Comments
 (0)