Skip to content

Commit e9fa13a

Browse files
committed
fix(imshow): validate slots so bad assignments raise a clear error
Two slot assignments reached px.imshow and crashed inside its own slicing with an error that named nothing the caller had written: - The same dimension in two slots popped the same index twice, giving "IndexError: pop index out of range". - Facet and animation slots consuming all but one dimension left nothing for the second heatmap axis, giving "IndexError: list index out of range". Duplicates that reached the transpose first surfaced as "ValueError: axes don't match array" instead. Validate the assignment before transposing: every slot needs its own dimension, and both y and x must be filled. The error now names the dimension and the two slots that want it, or the slots that consumed the axes. Sweeping all 3125 combinations of auto/None/each dimension across the five slots of a 4D array: every one now either builds a figure or raises a ValueError that says what to change, with no change to the 1056 combinations that already worked. Also drop mypy's python_version = "3.10" pin, which failed CI on the 3.12 and 3.13 matrix entries -- numpy's stubs now use `type` statements that mypy rejects when targeting 3.10. Reproduced on origin/main with no other changes. Unpinned, mypy targets the interpreter it runs under, so each matrix entry checks its own version against the numpy stubs resolved for it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TKhPQzJgQr7khmzDPYAXA9
1 parent 4c3e586 commit e9fa13a

3 files changed

Lines changed: 93 additions & 1 deletion

File tree

pyproject.toml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ ignore = [
9292
known-first-party = ["xarray_plotly"]
9393

9494
[tool.mypy]
95-
python_version = "3.10"
95+
# No python_version pin: mypy then targets the interpreter it runs under, so
96+
# each CI matrix entry checks its own version against the numpy stubs resolved
97+
# for it. Pinning 3.10 made mypy reject numpy's `type` statements outright.
9698
strict = true
9799
warn_return_any = true
98100
warn_unused_ignores = true

tests/test_accessor.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,51 @@ def test_imshow_facet_col_wrap_kept_without_facet_row(self) -> None:
547547
domains = {tuple(fig.layout[k].domain) for k in fig.layout if k.startswith("yaxis")}
548548
assert len(domains) == 2
549549

550+
def test_imshow_duplicate_dim_across_slots(self) -> None:
551+
"""Test a clear error when one dimension is asked to fill two slots.
552+
553+
px.imshow otherwise dies with "IndexError: pop index out of range".
554+
"""
555+
with pytest.raises(ValueError, match=r"'scenario' is assigned to both"):
556+
self.da_3d.plotly.imshow(y="lat", x="lon", facet_col="scenario", facet_row="scenario")
557+
558+
def test_imshow_duplicate_dim_facet_row_and_animation(self) -> None:
559+
"""Test the duplicate check across facet_row and animation_frame."""
560+
with pytest.raises(ValueError, match=r"'year' is assigned to both"):
561+
self.da_4d.plotly.imshow(
562+
y="lat", x="lon", facet_col="scenario", facet_row="year", animation_frame="year"
563+
)
564+
565+
def test_imshow_no_dimension_left_for_x(self) -> None:
566+
"""Test a clear error when facet/animation slots eat the heatmap axes.
567+
568+
px.imshow otherwise dies with "IndexError: list index out of range".
569+
"""
570+
with pytest.raises(ValueError, match=r"needs a dimension for both 'y' and 'x'"):
571+
self.da_4d.plotly.imshow(facet_col="lat", facet_row="lon", animation_frame="scenario")
572+
573+
def test_imshow_2d_with_both_facets_leaves_no_axes(self) -> None:
574+
"""Test the error when a 2D array puts both of its dims into facets."""
575+
da = xr.DataArray(
576+
np.random.rand(2, 3), dims=["a", "b"], coords={"a": [0, 1], "b": [0, 1, 2]}
577+
)
578+
with pytest.raises(ValueError, match=r"needs a dimension for both 'y' and 'x'"):
579+
da.plotly.imshow(facet_col="a", facet_row="b")
580+
581+
@requires_imshow_facet_row
582+
def test_imshow_explicit_x_y_facet_col_facet_row_4d(self) -> None:
583+
"""Test that naming all four slots on a 4D array builds the full grid."""
584+
fig = self.da_4d.plotly.imshow(x="lon", y="lat", facet_col="scenario", facet_row="year")
585+
assert len(fig.data) == 6
586+
facet_titles = {a.text for a in fig.layout.annotations if "=" in (a.text or "")}
587+
assert facet_titles == {
588+
"scenario=low",
589+
"scenario=high",
590+
"year=2020",
591+
"year=2021",
592+
"year=2022",
593+
}
594+
550595
@requires_imshow_facet_row
551596
def test_imshow_facet_grid_places_data_in_right_subplot(self) -> None:
552597
"""Test that each (facet_col, facet_row) pair lands in its own subplot."""

xarray_plotly/plotting.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,49 @@ def _imshow_supports_facet_row() -> bool:
671671
return "facet_row" in inspect.signature(px.imshow).parameters
672672

673673

674+
_IMSHOW_SLOTS = ("y", "x", "facet_col", "facet_row", "animation_frame")
675+
676+
677+
def _validate_imshow_slots(slots: dict[str, Hashable]) -> None:
678+
"""Check that imshow's slots form a usable heatmap before handing them to plotly.
679+
680+
Every imshow slot is a separate axis of the data, so each needs its own
681+
dimension and both heatmap axes must be filled. ``px.imshow`` does not
682+
check either, and fails deep inside its own slicing with ``IndexError:
683+
pop index out of range`` (a dimension used twice) or ``IndexError: list
684+
index out of range`` (nothing left for y/x).
685+
686+
Args:
687+
slots: Slot assignment from :func:`assign_slots`.
688+
689+
Raises:
690+
ValueError: If a dimension fills two slots, or y/x is left empty.
691+
"""
692+
seen: dict[Hashable, str] = {}
693+
for slot in _IMSHOW_SLOTS:
694+
dim = slots.get(slot)
695+
if dim is None:
696+
continue
697+
if dim in seen:
698+
msg = (
699+
f"Dimension {dim!r} is assigned to both {seen[dim]!r} and {slot!r}. "
700+
f"Each imshow slot needs its own dimension."
701+
)
702+
raise ValueError(msg)
703+
seen[dim] = slot
704+
705+
missing = [slot for slot in ("y", "x") if slots.get(slot) is None]
706+
if missing:
707+
taken = {slot: dim for dim, slot in seen.items()}
708+
msg = (
709+
f"imshow needs a dimension for both 'y' and 'x', but {missing} "
710+
f"came up empty; the other slots took {taken}. Free one with "
711+
f"facet_col=None, facet_row=None or animation_frame=None, or reduce "
712+
f"a dimension with .sel(), .isel() or .mean() before plotting."
713+
)
714+
raise ValueError(msg)
715+
716+
674717
def _handle_unsupported_facet_row(slots: dict[str, Hashable], *, explicit: bool) -> None:
675718
"""Resolve an imshow ``facet_row`` slot that the installed plotly cannot draw.
676719
@@ -788,6 +831,8 @@ def imshow(
788831
animation_frame=animation_frame,
789832
)
790833

834+
_validate_imshow_slots(slots)
835+
791836
if slots.get("facet_row") is not None and not _imshow_supports_facet_row():
792837
_handle_unsupported_facet_row(slots, explicit=facet_row is not auto)
793838

0 commit comments

Comments
 (0)