Skip to content

Commit 47ae7bc

Browse files
committed
refactor(as_points): trim comments, drop redundant asarray, fix empty-element warning
Code-review cleanup: lean the datashader as_points comments/docstrings, drop np.asarray on already-array x/y, and stop emitting the 'cannot use datashader for this colouring' warning for an empty element (it now warns only for the genuinely unsupported no-color case). Net -5 lines, no behavior change.
1 parent b18bb18 commit 47ae7bc

1 file changed

Lines changed: 16 additions & 21 deletions

File tree

src/spatialdata_plot/pl/render.py

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,9 +1111,8 @@ def _scatter_points(
11111111
)
11121112

11131113

1114-
# as_points: matplotlib draws crisp markers; above this many dots its per-glyph draw dominates
1115-
# (≈18 µs/dot), so auto-switch to datashader. Datashader changes appearance (density raster), so
1116-
# the threshold is conservative and only applies when the user did not pick a backend explicitly.
1114+
# Above this many centroids matplotlib's per-glyph draw (~18 µs/dot) dominates, so auto-switch to
1115+
# datashader. Conservative because datashader changes the look (density raster); only used when method=None.
11171116
AS_POINTS_DS_AUTO = 500_000
11181117

11191118

@@ -1123,9 +1122,9 @@ def _resolve_as_points_method(
11231122
"""Pick the as_points backend. matplotlib by default; datashader only when it can represent the colors."""
11241123
method = render_params.method
11251124
if not allow_datashader or n == 0:
1126-
# e.g. no-color labels get one distinct random colour per cell (`_map_color_seg` Case C),
1127-
# which datashader's aggregate-then-shade model cannot represent.
1128-
if method == "datashader":
1125+
# no-color labels get one random colour per cell (`_map_color_seg` Case C), which datashader's
1126+
# aggregate-then-shade model cannot represent; an empty element just has nothing to draw.
1127+
if method == "datashader" and not allow_datashader:
11291128
logger.warning("`as_points` cannot use datashader for this colouring; falling back to matplotlib.")
11301129
return "matplotlib"
11311130
if method == "datashader":
@@ -1157,16 +1156,15 @@ def _render_centroids_as_points(
11571156
colorbar_requests: list[ColorbarSpec] | None,
11581157
allow_datashader: bool = True,
11591158
) -> None:
1160-
"""Render one dot per cell at ``(x, y)`` colored like the fill, with legend/colorbar.
1159+
"""Render one dot per cell at ``(x, y)`` (coordinate-system coords), colored like the fill.
11611160
1162-
Shared "fast mode" draw for shapes/labels. ``x``/``y`` are in **coordinate-system coords** (so the
1163-
datashader canvas and matplotlib's ``transData`` agree). Backend is matplotlib unless
1164-
``render_params.method`` / the size threshold selects datashader (and the colouring supports it).
1165-
``norm``/``na_color`` stay explicit because they differ between the shapes and labels paths.
1161+
Shared "fast mode" for shapes/labels. Backend is matplotlib unless ``render_params.method`` or the
1162+
size threshold selects datashader (and the colouring supports it). ``norm``/``na_color`` are explicit
1163+
because they differ between the shapes and labels paths.
11661164
"""
1167-
method = _resolve_as_points_method(render_params, n=len(np.asarray(x)), allow_datashader=allow_datashader)
1165+
method = _resolve_as_points_method(render_params, n=len(x), allow_datashader=allow_datashader)
11681166
if method == "datashader":
1169-
df = pd.DataFrame({"x": np.asarray(x), "y": np.asarray(y)})
1167+
df = pd.DataFrame({"x": x, "y": y})
11701168
cax, color_vector, color_source_vector = _datashader_points(
11711169
ax,
11721170
df,
@@ -1235,16 +1233,13 @@ def _datashader_points(
12351233
fig_params: FigParams,
12361234
default_reduction: _DsReduction = "sum",
12371235
) -> tuple[Any, Any, Any]:
1238-
"""Datashade an x/y(+color) point frame onto ``ax``; returns ``(cax, color_vector, color_source_vector)``.
1236+
"""Datashade an x/y(+color) point frame onto ``ax``; return ``(cax, color_vector, color_source_vector)``.
12391237
1240-
Shared datashader draw for ``render_points`` and the centroid "fast mode" of shapes/labels. ``df``
1241-
holds ``x``/``y`` in coordinate-system coords (+ an optional color column). The (possibly
1242-
recomputed) color vectors are returned so the caller's legend/colorbar uses the same values.
1243-
Primitives are taken explicitly rather than a ``render_params`` because shapes/labels params use
1244-
``fill_alpha`` and lack the density fields.
1238+
Shared by ``render_points`` and the centroid "fast mode" of shapes/labels; ``df`` holds ``x``/``y``
1239+
in coordinate-system coords. The (possibly recomputed) color vectors are returned so the caller's
1240+
legend uses the same values. Primitives are explicit because shapes/labels params lack ``alpha``/density.
12451241
"""
1246-
# NOTE: s in matplotlib is in units of points**2; use dpi/100 so dpi!=100 still spreads correctly.
1247-
# Under density, spreading would smear the count signal across pixels, so disable it.
1242+
# spread radius from marker size (matplotlib points**2, dpi-scaled); off under density to keep counts crisp
12481243
px: int | None = None if density else int(np.round(np.sqrt(size) * (fig_params.fig.dpi / 100)))
12491244

12501245
plot_width, plot_height, x_ext, y_ext, factor = _datashader_canvas_from_dataframe(df, fig_params)

0 commit comments

Comments
 (0)