Skip to content

Commit 83a3c7d

Browse files
committed
refactor(show): tighten extracted helpers, cut ~70 LOC
Post-review cleanups (all behavior-preserving, full suite green 719 passed): - drop dead _draw_colorbar param base_offsets_axes (never read) - collapse the two parallel 4-branch location chains in _draw_colorbar into data-driven lookups (vertical flag + opposite map + getattr on the axis) - extract _should_rasterize() to dedup the images/labels rasterize heuristic - extract _maybe_set_label_colors() for the categorical-color prestep - replace the 5-branch render dispatch in _render_panel with a renderer table keyed by command; graph stays a small special case - pass cs_row instead of four has_* booleans; return a wants dict instead of a four-boolean tuple (removes the parallel-variable sprawl)
1 parent 6e70bc5 commit 83a3c7d

1 file changed

Lines changed: 82 additions & 153 deletions

File tree

‎src/spatialdata_plot/pl/basic.py‎

Lines changed: 82 additions & 153 deletions
Original file line numberDiff line numberDiff line change
@@ -1493,7 +1493,7 @@ def show(
14931493
axis_colorbar_requests: list[ColorbarSpec] | None = [] if legend_params_obj.colorbar else None
14941494
axis_channel_legend_entries: list[ChannelLegendEntry] = []
14951495

1496-
wanted_elements, wants_images, wants_labels, wants_points, wants_shapes = _render_panel(
1496+
wanted_elements, wants = _render_panel(
14971497
sdata=sdata,
14981498
render_cmds=render_cmds,
14991499
cs=cs,
@@ -1504,16 +1504,13 @@ def show(
15041504
legend_params_obj=legend_params_obj,
15051505
axis_colorbar_requests=axis_colorbar_requests,
15061506
axis_channel_legend_entries=axis_channel_legend_entries,
1507-
has_images=has_images,
1508-
has_labels=has_labels,
1509-
has_points=has_points,
1510-
has_shapes=has_shapes,
1507+
cs_row=cs_row,
15111508
title=title,
15121509
dpi=dpi,
15131510
figsize=figsize,
15141511
)
15151512

1516-
if has_shapes and wants_shapes:
1513+
if has_shapes and wants["shapes"]:
15171514
empty_shape_elements = [
15181515
name
15191516
for name in wanted_elements
@@ -1528,10 +1525,10 @@ def show(
15281525
extent = get_extent(
15291526
sdata,
15301527
coordinate_system=cs,
1531-
has_images=has_images and wants_images,
1532-
has_labels=has_labels and wants_labels,
1533-
has_points=has_points and wants_points,
1534-
has_shapes=has_shapes and wants_shapes,
1528+
has_images=has_images and wants["images"],
1529+
has_labels=has_labels and wants["labels"],
1530+
has_points=has_points and wants["points"],
1531+
has_shapes=has_shapes and wants["shapes"],
15351532
elements=wanted_elements,
15361533
)
15371534
cs_x_min, cs_x_max = extent["x"]
@@ -1767,7 +1764,6 @@ def _draw_colorbar(
17671764
spec: ColorbarSpec,
17681765
fig: Figure,
17691766
renderer: RendererBase,
1770-
base_offsets_axes: dict[str, float],
17711767
trackers_axes: dict[str, float],
17721768
colorbar_params: dict[str, object] | None,
17731769
) -> None:
@@ -1807,14 +1803,11 @@ def _draw_colorbar(
18071803
fraction = float(cast(float | int, layout.get("fraction", base_layout["fraction"])))
18081804
pad = float(cast(float | int, layout.get("pad", base_layout["pad"])))
18091805

1810-
if location in {"left", "right"}:
1811-
pad_axes = pad + trackers_axes[location]
1812-
x0 = -pad_axes - fraction if location == "left" else 1 + pad_axes
1813-
bbox = (float(x0), 0.0, float(fraction), 1.0)
1814-
else:
1815-
pad_axes = pad + trackers_axes[location]
1816-
y0 = -pad_axes - fraction if location == "bottom" else 1 + pad_axes
1817-
bbox = (0.0, float(y0), 1.0, float(fraction))
1806+
vertical = location in {"left", "right"}
1807+
pad_axes = pad + trackers_axes[location]
1808+
# "left"/"bottom" grow outward in the negative direction; "right"/"top" past 1.
1809+
start = -pad_axes - fraction if location in {"left", "bottom"} else 1 + pad_axes
1810+
bbox = (float(start), 0.0, float(fraction), 1.0) if vertical else (0.0, float(start), 1.0, float(fraction))
18181811
cax = inset_axes(
18191812
spec.ax,
18201813
width="100%",
@@ -1826,22 +1819,11 @@ def _draw_colorbar(
18261819
)
18271820

18281821
cb = fig.colorbar(spec.mappable, cax=cax, **cbar_kwargs)
1829-
if location == "left":
1830-
cb.ax.yaxis.set_ticks_position("left")
1831-
cb.ax.yaxis.set_label_position("left")
1832-
cb.ax.tick_params(labelleft=True, labelright=False)
1833-
elif location == "top":
1834-
cb.ax.xaxis.set_ticks_position("top")
1835-
cb.ax.xaxis.set_label_position("top")
1836-
cb.ax.tick_params(labeltop=True, labelbottom=False)
1837-
elif location == "right":
1838-
cb.ax.yaxis.set_ticks_position("right")
1839-
cb.ax.yaxis.set_label_position("right")
1840-
cb.ax.tick_params(labelright=True, labelleft=False)
1841-
elif location == "bottom":
1842-
cb.ax.xaxis.set_ticks_position("bottom")
1843-
cb.ax.xaxis.set_label_position("bottom")
1844-
cb.ax.tick_params(labelbottom=True, labeltop=False)
1822+
opposite = {"left": "right", "right": "left", "top": "bottom", "bottom": "top"}[location]
1823+
cbar_axis = cb.ax.yaxis if vertical else cb.ax.xaxis
1824+
cbar_axis.set_ticks_position(location)
1825+
cbar_axis.set_label_position(location)
1826+
cb.ax.tick_params(**{f"label{location}": True, f"label{opposite}": False})
18451827

18461828
final_label = global_label_override or layer_label_override or spec.label
18471829
if final_label:
@@ -1850,14 +1832,7 @@ def _draw_colorbar(
18501832
with contextlib.suppress(Exception):
18511833
cb.solids.set_alpha(spec.alpha)
18521834
bbox_axes = cb.ax.get_tightbbox(renderer).transformed(spec.ax.transAxes.inverted())
1853-
if location == "left":
1854-
trackers_axes["left"] = pad_axes + bbox_axes.width
1855-
elif location == "right":
1856-
trackers_axes["right"] = pad_axes + bbox_axes.width
1857-
elif location == "bottom":
1858-
trackers_axes["bottom"] = pad_axes + bbox_axes.height
1859-
elif location == "top":
1860-
trackers_axes["top"] = pad_axes + bbox_axes.height
1835+
trackers_axes[location] = pad_axes + (bbox_axes.width if vertical else bbox_axes.height)
18611836

18621837

18631838
def _layout_pending_colorbars(
@@ -1887,15 +1862,14 @@ def _layout_pending_colorbars(
18871862
seen_mappables.add(mappable_id)
18881863
unique_specs.append(spec)
18891864
tight_bbox = axis.get_tightbbox(renderer).transformed(axis.transAxes.inverted())
1890-
base_offsets_axes = {
1865+
trackers_axes = {
18911866
"left": max(0.0, -tight_bbox.x0),
18921867
"right": max(0.0, tight_bbox.x1 - 1),
18931868
"bottom": max(0.0, -tight_bbox.y0),
18941869
"top": max(0.0, tight_bbox.y1 - 1),
18951870
}
1896-
trackers_axes = {k: base_offsets_axes[k] for k in base_offsets_axes}
18971871
for spec in unique_specs:
1898-
_draw_colorbar(spec, fig, renderer, base_offsets_axes, trackers_axes, colorbar_params)
1872+
_draw_colorbar(spec, fig, renderer, trackers_axes, colorbar_params)
18991873

19001874

19011875
def _finalize_panel(
@@ -1927,6 +1901,31 @@ def _finalize_panel(
19271901
ax.axis("off")
19281902

19291903

1904+
def _should_rasterize(
1905+
render_params: ImageRenderParams | LabelsRenderParams,
1906+
dpi: int | None,
1907+
figsize: tuple[float, float] | None,
1908+
) -> bool:
1909+
"""Rasterize when no scale is set, or a non-``"full"`` scale is paired with a fixed canvas size."""
1910+
scale = render_params.scale
1911+
return scale is None or (isinstance(scale, str) and scale != "full" and (dpi is not None or figsize is not None))
1912+
1913+
1914+
def _maybe_set_label_colors(sdata: sd.SpatialData, render_params: LabelsRenderParams) -> None:
1915+
"""Materialize a categorical palette on the table annotating a labels element, if applicable."""
1916+
table = render_params.table_name
1917+
if table is None or render_params.col_for_color is None:
1918+
return
1919+
colors = sc.get.obs_df(sdata[table], [render_params.col_for_color])
1920+
if isinstance(colors[render_params.col_for_color].dtype, pd.CategoricalDtype):
1921+
_maybe_set_colors(
1922+
source=sdata[table],
1923+
target=sdata[table],
1924+
key=render_params.col_for_color,
1925+
palette=render_params.palette,
1926+
)
1927+
1928+
19301929
def _render_panel(
19311930
sdata: sd.SpatialData,
19321931
render_cmds: list[_RenderCmd],
@@ -1938,25 +1937,26 @@ def _render_panel(
19381937
legend_params_obj: LegendParams,
19391938
axis_colorbar_requests: list[ColorbarSpec] | None,
19401939
axis_channel_legend_entries: list[ChannelLegendEntry],
1941-
has_images: bool,
1942-
has_labels: bool,
1943-
has_points: bool,
1944-
has_shapes: bool,
1940+
cs_row: pd.Series,
19451941
title: list[str] | None,
19461942
dpi: int | None,
19471943
figsize: tuple[float, float] | None,
1948-
) -> tuple[list[str], bool, bool, bool, bool]:
1944+
) -> tuple[list[str], dict[str, bool]]:
19491945
"""Render every applicable render command into a single panel's axes.
19501946
19511947
Dispatches each queued render command to its ``_render_*`` function, skipping entries that
19521948
belong to a different color panel (``panel_key``). Colorbar requests and channel-legend
1953-
entries accumulate on the passed-in lists. Returns the wanted element names and the per-type
1954-
``wants_*`` flags needed downstream for extent computation.
1949+
entries accumulate on the passed-in lists. Returns the wanted element names and a per-type
1950+
``wants`` flag dict (keyed ``"images"``/``"labels"``/``"points"``/``"shapes"``) needed
1951+
downstream for extent computation.
19551952
"""
1956-
wants_images = False
1957-
wants_labels = False
1958-
wants_points = False
1959-
wants_shapes = False
1953+
renderers = {
1954+
"render_images": _render_images,
1955+
"render_shapes": _render_shapes,
1956+
"render_points": _render_points,
1957+
"render_labels": _render_labels,
1958+
}
1959+
wants = dict.fromkeys(("images", "labels", "points", "shapes"), False)
19601960
wanted_elements: list[str] = []
19611961

19621962
for cmd, params in render_cmds:
@@ -1967,103 +1967,10 @@ def _render_panel(
19671967
continue
19681968
# We create a copy here as the wanted elements can change from one cs to another.
19691969
params_copy = deepcopy(params)
1970-
if cmd == "render_images" and has_images:
1971-
wanted_elements, wanted_images_on_this_cs, wants_images = _get_wanted_render_elements(
1972-
sdata, wanted_elements, params_copy, cs, "images"
1973-
)
1974-
1975-
if wanted_images_on_this_cs:
1976-
rasterize = (params_copy.scale is None) or (
1977-
isinstance(params_copy.scale, str)
1978-
and params_copy.scale != "full"
1979-
and (dpi is not None or figsize is not None)
1980-
)
1981-
_render_images(
1982-
sdata=sdata,
1983-
render_params=params_copy,
1984-
coordinate_system=cs,
1985-
ax=ax,
1986-
fig_params=fig_params,
1987-
legend_params=legend_params_obj,
1988-
colorbar_requests=axis_colorbar_requests,
1989-
channel_legend_entries=axis_channel_legend_entries,
1990-
rasterize=rasterize,
1991-
)
1992-
1993-
elif cmd == "render_shapes" and has_shapes:
1994-
wanted_elements, wanted_shapes_on_this_cs, wants_shapes = _get_wanted_render_elements(
1995-
sdata, wanted_elements, params_copy, cs, "shapes"
1996-
)
19971970

1998-
if wanted_shapes_on_this_cs:
1999-
_render_shapes(
2000-
sdata=sdata,
2001-
render_params=params_copy,
2002-
coordinate_system=cs,
2003-
ax=ax,
2004-
fig_params=fig_params,
2005-
legend_params=legend_params_obj,
2006-
colorbar_requests=axis_colorbar_requests,
2007-
)
2008-
2009-
elif cmd == "render_points" and has_points:
2010-
wanted_elements, wanted_points_on_this_cs, wants_points = _get_wanted_render_elements(
2011-
sdata, wanted_elements, params_copy, cs, "points"
2012-
)
2013-
2014-
if wanted_points_on_this_cs:
2015-
_render_points(
2016-
sdata=sdata,
2017-
render_params=params_copy,
2018-
coordinate_system=cs,
2019-
ax=ax,
2020-
fig_params=fig_params,
2021-
legend_params=legend_params_obj,
2022-
colorbar_requests=axis_colorbar_requests,
2023-
)
2024-
2025-
elif cmd == "render_labels" and has_labels:
2026-
wanted_elements, wanted_labels_on_this_cs, wants_labels = _get_wanted_render_elements(
2027-
sdata, wanted_elements, params_copy, cs, "labels"
2028-
)
2029-
2030-
if wanted_labels_on_this_cs:
2031-
table = params_copy.table_name
2032-
if table is not None and params_copy.col_for_color is not None:
2033-
colors = sc.get.obs_df(sdata[table], [params_copy.col_for_color])
2034-
if isinstance(
2035-
colors[params_copy.col_for_color].dtype,
2036-
pd.CategoricalDtype,
2037-
):
2038-
_maybe_set_colors(
2039-
source=sdata[table],
2040-
target=sdata[table],
2041-
key=params_copy.col_for_color,
2042-
palette=params_copy.palette,
2043-
)
2044-
2045-
rasterize = (params_copy.scale is None) or (
2046-
isinstance(params_copy.scale, str)
2047-
and params_copy.scale != "full"
2048-
and (dpi is not None or figsize is not None)
2049-
)
2050-
_render_labels(
2051-
sdata=sdata,
2052-
render_params=params_copy,
2053-
coordinate_system=cs,
2054-
ax=ax,
2055-
fig_params=fig_params,
2056-
legend_params=legend_params_obj,
2057-
colorbar_requests=axis_colorbar_requests,
2058-
rasterize=rasterize,
2059-
)
2060-
2061-
elif cmd == "render_graph":
1971+
if cmd == "render_graph":
20621972
graph_element = params_copy.element
2063-
element_in_cs = graph_element in sdata and cs in set(
2064-
get_transformation(sdata[graph_element], get_all=True).keys()
2065-
)
2066-
if element_in_cs:
1973+
if graph_element in sdata and cs in get_transformation(sdata[graph_element], get_all=True):
20671974
_render_graph(
20681975
sdata=sdata,
20691976
render_params=params_copy,
@@ -2072,7 +1979,29 @@ def _render_panel(
20721979
legend_params=legend_params_obj,
20731980
colorbar_requests=axis_colorbar_requests,
20741981
)
1982+
elif cmd in renderers and cs_row[_RENDER_CMD_TO_CS_FLAG[cmd]]:
1983+
element_type = cmd.removeprefix("render_")
1984+
wanted_elements, wanted_on_cs, wants[element_type] = _get_wanted_render_elements(
1985+
sdata, wanted_elements, params_copy, cs, element_type
1986+
)
1987+
if wanted_on_cs:
1988+
kwargs: dict[str, Any] = {
1989+
"sdata": sdata,
1990+
"render_params": params_copy,
1991+
"coordinate_system": cs,
1992+
"ax": ax,
1993+
"fig_params": fig_params,
1994+
"legend_params": legend_params_obj,
1995+
"colorbar_requests": axis_colorbar_requests,
1996+
}
1997+
if cmd == "render_images":
1998+
kwargs["channel_legend_entries"] = axis_channel_legend_entries
1999+
if cmd in {"render_images", "render_labels"}:
2000+
kwargs["rasterize"] = _should_rasterize(params_copy, dpi, figsize)
2001+
if cmd == "render_labels":
2002+
_maybe_set_label_colors(sdata, params_copy)
2003+
renderers[cmd](**kwargs)
20752004

20762005
_finalize_panel(ax, panel_idx, title, panel_key, cs, fig_params.frameon)
20772006

2078-
return wanted_elements, wants_images, wants_labels, wants_points, wants_shapes
2007+
return wanted_elements, wants

0 commit comments

Comments
 (0)