Skip to content

Commit b9d004e

Browse files
committed
Raise on obs/var key collision in table-based coloring
When the same key existed in both `table.obs.columns` and `table.var_names`, upstream `_get_table_origins` returned only the obs origin (elif chain), silently masking gene expression with obs values. Catch the collision at the renderer entry points and raise a descriptive ValueError naming both locations so the user can disambiguate. Fixes #621
1 parent 3ebefe1 commit b9d004e

3 files changed

Lines changed: 86 additions & 1 deletion

File tree

src/spatialdata_plot/pl/render.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
)
5959
from spatialdata_plot.pl.utils import (
6060
_ax_show_and_transform,
61+
_check_obs_var_shadow,
6162
_convert_shapes,
6263
_datashader_canvas_from_dataframe,
6364
_decorate_axs,
@@ -370,6 +371,8 @@ def _render_shapes(
370371
groups = render_params.groups
371372
table_layer = render_params.table_layer
372373

374+
_check_obs_var_shadow(sdata, element, col_for_color, render_params.table_name)
375+
373376
sdata_filt = sdata.filter_by_coordinate_system(
374377
coordinate_system=coordinate_system,
375378
filter_tables=bool(render_params.table_name),
@@ -766,6 +769,8 @@ def _render_points(
766769
groups = render_params.groups
767770
palette = render_params.palette
768771

772+
_check_obs_var_shadow(sdata, element, col_for_color, table_name)
773+
769774
if isinstance(groups, str):
770775
groups = [groups]
771776

@@ -1687,6 +1692,8 @@ def _render_labels(
16871692
groups = render_params.groups
16881693
scale = render_params.scale
16891694

1695+
_check_obs_var_shadow(sdata, element, col_for_color, table_name)
1696+
16901697
sdata_filt = sdata.filter_by_coordinate_system(
16911698
coordinate_system=coordinate_system,
16921699
filter_tables=bool(table_name),

src/spatialdata_plot/pl/utils.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
)
6565
from spatialdata._core.query.relational_query import _locate_value
6666
from spatialdata._types import ArrayLike
67-
from spatialdata.models import Image2DModel, Labels2DModel, SpatialElement, get_table_keys
67+
from spatialdata.models import Image2DModel, Labels2DModel, SpatialElement, TableModel, get_table_keys
6868
from spatialdata.transformations.operations import get_transformation
6969
from spatialdata.transformations.transformations import Scale, Translation
7070
from spatialdata.transformations.transformations import Sequence as TransformSequence
@@ -104,6 +104,34 @@
104104
}
105105

106106

107+
def _check_obs_var_shadow(
108+
sdata: SpatialData | None,
109+
element_name: str | None,
110+
value_to_plot: str | None,
111+
table_name: str | None,
112+
) -> None:
113+
"""Raise if `value_to_plot` exists in both `table.obs.columns` and `table.var_names`.
114+
115+
Upstream `_get_table_origins` uses an `elif` chain, so a key that lives in both
116+
locations is silently resolved to `obs` — masking the user's likely intent of
117+
plotting gene expression. Catch this here before any value fetch.
118+
"""
119+
if value_to_plot is None or table_name is None or sdata is None or table_name not in sdata.tables:
120+
return
121+
table = sdata.tables[table_name]
122+
if value_to_plot not in table.obs.columns or value_to_plot not in table.var_names:
123+
return
124+
region = table.uns[TableModel.ATTRS_KEY][TableModel.REGION_KEY]
125+
annotates_element = element_name in region if isinstance(region, list) else element_name == region
126+
if not annotates_element:
127+
return
128+
raise ValueError(
129+
f"Color key '{value_to_plot}' is ambiguous: it exists in both "
130+
f"`table['{table_name}'].obs.columns` and `table['{table_name}'].var_names`. "
131+
"Rename one of them (or drop the obs column) so the intended source is unambiguous."
132+
)
133+
134+
107135
def _gate_palette_and_groups(
108136
element_params: dict[str, Any],
109137
param_dict: dict[str, Any],

tests/pl/test_utils.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,56 @@ def test_color_column_collision_on_annotating_table_raises():
422422
sdata.pl.render_shapes("s", color="#ffa500")
423423

424424

425+
def test_color_key_obs_var_shadow_raises():
426+
# regression test for #621: when the same key exists in both `table.obs.columns`
427+
# and `table.var_names`, upstream `_get_table_origins` silently returns only the
428+
# obs origin (elif chain). Surface the ambiguity at the plotting layer instead.
429+
pts = PointsModel.parse(pd.DataFrame({"x": [1.0, 2.0, 3.0, 4.0], "y": [1.0, 2.0, 3.0, 4.0]}))
430+
obs = pd.DataFrame(
431+
{
432+
"instance_id": [0, 1, 2, 3],
433+
"region": ["pts"] * 4,
434+
"GeneA": [0.9, 0.8, 0.7, 0.6],
435+
}
436+
)
437+
obs.index = obs.index.astype(str)
438+
X = np.array([[1.0, 0.5], [0.8, 0.2], [0.3, 0.9], [0.1, 0.7]])
439+
table = TableModel.parse(
440+
AnnData(X=X, obs=obs, var=pd.DataFrame(index=["GeneA", "GeneB"])),
441+
region=["pts"],
442+
region_key="region",
443+
instance_key="instance_id",
444+
)
445+
sdata = SpatialData(points={"pts": pts}, tables={"t": table})
446+
447+
with pytest.raises(ValueError, match=r"'GeneA'.*ambiguous.*obs\.columns.*var_names"):
448+
sdata.pl.render_points("pts", color="GeneA", table_name="t").pl.show()
449+
450+
# Negative control: only in var → no error.
451+
obs_var_only = pd.DataFrame(
452+
{"instance_id": [0, 1, 2, 3], "region": ["pts"] * 4},
453+
index=[str(i) for i in range(4)],
454+
)
455+
table_var_only = TableModel.parse(
456+
AnnData(X=X, obs=obs_var_only, var=pd.DataFrame(index=["GeneA", "GeneB"])),
457+
region=["pts"],
458+
region_key="region",
459+
instance_key="instance_id",
460+
)
461+
sdata_var_only = SpatialData(points={"pts": pts}, tables={"t": table_var_only})
462+
sdata_var_only.pl.render_points("pts", color="GeneA", table_name="t").pl.show()
463+
464+
# Negative control: only in obs → no error.
465+
table_obs_only = TableModel.parse(
466+
AnnData(X=X, obs=obs, var=pd.DataFrame(index=["G1", "G2"])),
467+
region=["pts"],
468+
region_key="region",
469+
instance_key="instance_id",
470+
)
471+
sdata_obs_only = SpatialData(points={"pts": pts}, tables={"t": table_obs_only})
472+
sdata_obs_only.pl.render_points("pts", color="GeneA", table_name="t").pl.show()
473+
474+
425475
def test_explicit_table_name_honored_when_element_has_same_column():
426476
# regression test for #620: explicit table_name= must not be silently
427477
# discarded when the element has a same-named column with different values.

0 commit comments

Comments
 (0)