From fc796d6bd665bb5ce4f10cdef1279506fc944e54 Mon Sep 17 00:00:00 2001 From: anon Date: Thu, 14 May 2026 15:24:34 +0200 Subject: [PATCH 1/3] Raise on color name collision with column in render_* (#619) When `color=` was both a valid matplotlib color and a column name in the element or an annotating table, the literal color silently won and the column was ignored (only a default-hidden `logger.info` was emitted). Now raise `ValueError` with disambiguation guidance: pass hex (e.g. `"#ffa500"`), an RGB(A) tuple, or rename the column. --- src/spatialdata_plot/pl/basic.py | 4 +++- src/spatialdata_plot/pl/utils.py | 38 +++++++++++++++++++++++++++++++- tests/pl/test_render_labels.py | 26 ++++++++++++++++++++++ tests/pl/test_render_points.py | 13 +++++++++++ tests/pl/test_render_shapes.py | 27 +++++++++++++++++++++++ 5 files changed, 106 insertions(+), 2 deletions(-) diff --git a/src/spatialdata_plot/pl/basic.py b/src/spatialdata_plot/pl/basic.py index 38183fa8..e1d28d3b 100644 --- a/src/spatialdata_plot/pl/basic.py +++ b/src/spatialdata_plot/pl/basic.py @@ -293,7 +293,9 @@ def render_shapes( ----- - Empty geometries will be removed at the time of plotting. - An `outline_width` of 0.0 leads to no border being plotted. - - When passing a color-like to 'color', this has precedence over the potential existence as a column name. + - If ``color`` is a string that is both a matplotlib color name and a column name in the + element or an annotating table, a ``ValueError`` is raised. Disambiguate by passing + a hex string (e.g. ``"#ffa500"``) or an RGB(A) tuple, or by renaming the column. Returns ------- diff --git a/src/spatialdata_plot/pl/utils.py b/src/spatialdata_plot/pl/utils.py index 4e872c8f..a9f7eef4 100644 --- a/src/spatialdata_plot/pl/utils.py +++ b/src/spatialdata_plot/pl/utils.py @@ -2280,6 +2280,41 @@ def _validate_show_parameters( ) +def _check_color_column_collision( + sdata: SpatialData, + elements: list[str], + color: str, + element_type: str, +) -> None: + """Raise if ``color`` is a color-like string that also names a column in the element or its tables.""" + matches: list[str] = [] + for el in elements: + if element_type in {"shapes", "points"}: + try: + el_cols = sdata[el].columns + except (KeyError, AttributeError): + el_cols = () + if color in el_cols: + matches.append(f"element '{el}'") + continue + try: + tables = get_element_annotators(sdata, el) + except (KeyError, ValueError): + tables = set() + for t in tables: + adata = sdata[t] + if color in adata.obs.columns or color in adata.var_names: + matches.append(f"table '{t}' (annotating '{el}')") + break + if matches: + locations = ", ".join(sorted(set(matches))) + raise ValueError( + f"`color={color!r}` is ambiguous: it is a valid matplotlib color name AND a column " + f"name in {locations}. Disambiguate by either passing an unambiguous color form " + f"(hex string like '#ffa500' or an RGB(A) tuple), or by renaming the column." + ) + + def _type_check_params(param_dict: dict[str, Any], element_type: str) -> dict[str, Any]: colorbar = param_dict.get("colorbar", "auto") if colorbar not in {True, False, None, "auto"}: @@ -2330,7 +2365,8 @@ def _type_check_params(param_dict: dict[str, Any], element_type: str) -> dict[st if not isinstance(color, str | tuple | list): raise TypeError("Parameter 'color' must be a string or a tuple/list of floats.") if _is_color_like(color): - logger.info("Value for parameter 'color' appears to be a color, using it as such.") + if isinstance(color, str): + _check_color_column_collision(param_dict["sdata"], param_dict["element"], color, element_type) param_dict["col_for_color"] = None param_dict["color"] = Color(color) if param_dict["color"].alpha_is_user_defined(): diff --git a/tests/pl/test_render_labels.py b/tests/pl/test_render_labels.py index 931ac24c..c548cacd 100644 --- a/tests/pl/test_render_labels.py +++ b/tests/pl/test_render_labels.py @@ -550,3 +550,29 @@ def test_render_labels_disjoint_instance_ids_clear_error(): sdata.pl.render_labels("lbl", color="cat", table_name="t").pl.show(ax=ax) finally: plt.close(fig) + + +def test_render_labels_color_column_name_collision_raises(): + # regression test for #619: color="orange" + obs column "orange" must raise. + arr = np.zeros((10, 10), dtype=np.int32) + arr[2:5, 2:5] = 1 + arr[6:9, 6:9] = 2 + obs = pd.DataFrame( + { + "region": pd.Categorical(["lbl"] * 2), + "instance_id": [1, 2], + "orange": pd.Categorical(["A", "B"]), + } + ) + table = TableModel.parse( + AnnData(X=np.zeros((2, 1)), obs=obs), + region="lbl", + region_key="region", + instance_key="instance_id", + ) + sdata = SpatialData(labels={"lbl": Labels2DModel.parse(arr, dims=["y", "x"])}, tables={"t": table}) + + with pytest.raises(ValueError, match=r"color='orange'.*ambiguous.*column"): + sdata.pl.render_labels("lbl", color="orange", table_name="t") + + sdata.pl.render_labels("lbl", color="#ffa500", table_name="t") diff --git a/tests/pl/test_render_points.py b/tests/pl/test_render_points.py index dcc4267c..bad7875f 100644 --- a/tests/pl/test_render_points.py +++ b/tests/pl/test_render_points.py @@ -1031,3 +1031,16 @@ def test_render_points_disjoint_instance_ids_clear_error(): sdata.pl.render_points("pts", color="cat", table_name="t").pl.show(ax=ax) finally: plt.close(fig) + + +def test_render_points_color_column_name_collision_raises(): + # regression test for #619: color="orange" + element column "orange" must raise. + points = PointsModel.parse( + pd.DataFrame({"x": [1.0, 2.0, 3.0, 4.0], "y": [1.0, 2.0, 3.0, 4.0], "orange": [0.1, 0.2, 0.3, 0.4]}) + ) + sdata = SpatialData(points={"pts": points}) + + with pytest.raises(ValueError, match=r"color='orange'.*ambiguous.*column"): + sdata.pl.render_points("pts", color="orange") + + sdata.pl.render_points("pts", color="#ffa500") diff --git a/tests/pl/test_render_shapes.py b/tests/pl/test_render_shapes.py index a401fef8..93727944 100644 --- a/tests/pl/test_render_shapes.py +++ b/tests/pl/test_render_shapes.py @@ -1378,6 +1378,33 @@ def test_render_shapes_disjoint_instance_ids_clear_error(): plt.close(fig) +def test_render_shapes_color_column_name_collision_raises(): + # regression test for #619: color="orange" + column "orange" must raise rather than silently + # treat the value as a literal color and shadow the column. + n = 4 + shapes = ShapesModel.parse(gpd.GeoDataFrame({"geometry": [Point(i, 0) for i in range(n)], "radius": [0.5] * n})) + obs = pd.DataFrame( + { + "region": pd.Categorical(["s"] * n), + "instance_id": list(range(n)), + "orange": pd.Categorical(["A", "B", "A", "B"]), + } + ) + table = TableModel.parse( + AnnData(X=np.zeros((n, 1)), obs=obs), + region="s", + region_key="region", + instance_key="instance_id", + ) + sdata = SpatialData(shapes={"s": shapes}, tables={"t": table}) + + with pytest.raises(ValueError, match=r"color='orange'.*ambiguous.*column"): + sdata.pl.render_shapes("s", color="orange") + + sdata.pl.render_shapes("s", color="#ffa500") + sdata.pl.render_shapes("s", color=(1.0, 0.65, 0.0)) + + def test_datashader_colorbar_range_matches_data(sdata_blobs: SpatialData): """Datashader colorbar range must not exceed the actual data range for shapes. From 970a21b23ef5c997cc876355c51306d97d3ef1b0 Mon Sep 17 00:00:00 2001 From: anon Date: Thu, 14 May 2026 15:30:12 +0200 Subject: [PATCH 2/3] Consolidate #619 regression tests into test_utils.py The three per-render-function tests all routed through the same `_type_check_params` -> `_check_color_column_collision` call. Collapse to two tests in `test_utils.py`, one per code path inside the helper: element-column lookup (points) and annotating-table lookup (shapes). --- tests/pl/test_render_labels.py | 26 ---------------------- tests/pl/test_render_points.py | 13 ----------- tests/pl/test_render_shapes.py | 27 ----------------------- tests/pl/test_utils.py | 40 ++++++++++++++++++++++++++++++++++ 4 files changed, 40 insertions(+), 66 deletions(-) diff --git a/tests/pl/test_render_labels.py b/tests/pl/test_render_labels.py index c548cacd..931ac24c 100644 --- a/tests/pl/test_render_labels.py +++ b/tests/pl/test_render_labels.py @@ -550,29 +550,3 @@ def test_render_labels_disjoint_instance_ids_clear_error(): sdata.pl.render_labels("lbl", color="cat", table_name="t").pl.show(ax=ax) finally: plt.close(fig) - - -def test_render_labels_color_column_name_collision_raises(): - # regression test for #619: color="orange" + obs column "orange" must raise. - arr = np.zeros((10, 10), dtype=np.int32) - arr[2:5, 2:5] = 1 - arr[6:9, 6:9] = 2 - obs = pd.DataFrame( - { - "region": pd.Categorical(["lbl"] * 2), - "instance_id": [1, 2], - "orange": pd.Categorical(["A", "B"]), - } - ) - table = TableModel.parse( - AnnData(X=np.zeros((2, 1)), obs=obs), - region="lbl", - region_key="region", - instance_key="instance_id", - ) - sdata = SpatialData(labels={"lbl": Labels2DModel.parse(arr, dims=["y", "x"])}, tables={"t": table}) - - with pytest.raises(ValueError, match=r"color='orange'.*ambiguous.*column"): - sdata.pl.render_labels("lbl", color="orange", table_name="t") - - sdata.pl.render_labels("lbl", color="#ffa500", table_name="t") diff --git a/tests/pl/test_render_points.py b/tests/pl/test_render_points.py index bad7875f..dcc4267c 100644 --- a/tests/pl/test_render_points.py +++ b/tests/pl/test_render_points.py @@ -1031,16 +1031,3 @@ def test_render_points_disjoint_instance_ids_clear_error(): sdata.pl.render_points("pts", color="cat", table_name="t").pl.show(ax=ax) finally: plt.close(fig) - - -def test_render_points_color_column_name_collision_raises(): - # regression test for #619: color="orange" + element column "orange" must raise. - points = PointsModel.parse( - pd.DataFrame({"x": [1.0, 2.0, 3.0, 4.0], "y": [1.0, 2.0, 3.0, 4.0], "orange": [0.1, 0.2, 0.3, 0.4]}) - ) - sdata = SpatialData(points={"pts": points}) - - with pytest.raises(ValueError, match=r"color='orange'.*ambiguous.*column"): - sdata.pl.render_points("pts", color="orange") - - sdata.pl.render_points("pts", color="#ffa500") diff --git a/tests/pl/test_render_shapes.py b/tests/pl/test_render_shapes.py index 93727944..a401fef8 100644 --- a/tests/pl/test_render_shapes.py +++ b/tests/pl/test_render_shapes.py @@ -1378,33 +1378,6 @@ def test_render_shapes_disjoint_instance_ids_clear_error(): plt.close(fig) -def test_render_shapes_color_column_name_collision_raises(): - # regression test for #619: color="orange" + column "orange" must raise rather than silently - # treat the value as a literal color and shadow the column. - n = 4 - shapes = ShapesModel.parse(gpd.GeoDataFrame({"geometry": [Point(i, 0) for i in range(n)], "radius": [0.5] * n})) - obs = pd.DataFrame( - { - "region": pd.Categorical(["s"] * n), - "instance_id": list(range(n)), - "orange": pd.Categorical(["A", "B", "A", "B"]), - } - ) - table = TableModel.parse( - AnnData(X=np.zeros((n, 1)), obs=obs), - region="s", - region_key="region", - instance_key="instance_id", - ) - sdata = SpatialData(shapes={"s": shapes}, tables={"t": table}) - - with pytest.raises(ValueError, match=r"color='orange'.*ambiguous.*column"): - sdata.pl.render_shapes("s", color="orange") - - sdata.pl.render_shapes("s", color="#ffa500") - sdata.pl.render_shapes("s", color=(1.0, 0.65, 0.0)) - - def test_datashader_colorbar_range_matches_data(sdata_blobs: SpatialData): """Datashader colorbar range must not exceed the actual data range for shapes. diff --git a/tests/pl/test_utils.py b/tests/pl/test_utils.py index a648b3d8..467a5efe 100644 --- a/tests/pl/test_utils.py +++ b/tests/pl/test_utils.py @@ -1,3 +1,4 @@ +import geopandas as gpd import matplotlib import matplotlib.pyplot as plt import numpy as np @@ -5,7 +6,10 @@ import pytest import scanpy as sc import xarray as xr +from anndata import AnnData +from shapely.geometry import Point from spatialdata import SpatialData +from spatialdata.models import PointsModel, ShapesModel, TableModel import spatialdata_plot from spatialdata_plot.pl.render_params import Color @@ -380,3 +384,39 @@ def test_exact_match_selects_that_scale(self): multiscale = self._make_multiscale((3, 500, 500), [2, 2]) result = _multiscale_to_spatial_image(multiscale, dpi=100.0, width=2.5, height=2.5) assert result.sizes["x"] == 250 + + +def test_color_column_collision_on_element_columns_raises(): + # regression test for #619, element-column path: points with an "orange" column + color="orange". + points = PointsModel.parse(pd.DataFrame({"x": [1.0, 2.0, 3.0], "y": [1.0, 2.0, 3.0], "orange": [0.1, 0.2, 0.3]})) + sdata = SpatialData(points={"pts": points}) + + with pytest.raises(ValueError, match=r"color='orange'.*ambiguous.*element 'pts'"): + sdata.pl.render_points("pts", color="orange") + + sdata.pl.render_points("pts", color="#ffa500") + sdata.pl.render_points("pts", color=(1.0, 0.65, 0.0)) + + +def test_color_column_collision_on_annotating_table_raises(): + # regression test for #619, table path: element has no "orange" column but its annotating table does. + shapes = ShapesModel.parse(gpd.GeoDataFrame({"geometry": [Point(i, 0) for i in range(3)], "radius": [0.5] * 3})) + obs = pd.DataFrame( + { + "region": pd.Categorical(["s"] * 3), + "instance_id": list(range(3)), + "orange": pd.Categorical(["A", "B", "A"]), + } + ) + table = TableModel.parse( + AnnData(X=np.zeros((3, 1)), obs=obs), + region="s", + region_key="region", + instance_key="instance_id", + ) + sdata = SpatialData(shapes={"s": shapes}, tables={"t": table}) + + with pytest.raises(ValueError, match=r"color='orange'.*ambiguous.*table 't'"): + sdata.pl.render_shapes("s", color="orange") + + sdata.pl.render_shapes("s", color="#ffa500") From 5839770876a6c794c10b28d96e0ab29b28b6554c Mon Sep 17 00:00:00 2001 From: anon Date: Thu, 14 May 2026 15:32:20 +0200 Subject: [PATCH 3/3] =?UTF-8?q?Drop=20redundant=20set()=20=E2=80=94=20coll?= =?UTF-8?q?ision=20matches=20are=20unique=20by=20construction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/spatialdata_plot/pl/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spatialdata_plot/pl/utils.py b/src/spatialdata_plot/pl/utils.py index a9f7eef4..dd0daee6 100644 --- a/src/spatialdata_plot/pl/utils.py +++ b/src/spatialdata_plot/pl/utils.py @@ -2307,7 +2307,7 @@ def _check_color_column_collision( matches.append(f"table '{t}' (annotating '{el}')") break if matches: - locations = ", ".join(sorted(set(matches))) + locations = ", ".join(matches) raise ValueError( f"`color={color!r}` is ambiguous: it is a valid matplotlib color name AND a column " f"name in {locations}. Disambiguate by either passing an unambiguous color form "