fix(imshow): make faceted slot assignments fail loudly and legibly

pyproject.toml

@@ -92,7 +92,9 @@ ignore = [
 known-first-party = ["xarray_plotly"]
 
 [tool.mypy]
-python_version = "3.10"
+# No python_version pin: mypy then targets the interpreter it runs under, so
+# each CI matrix entry checks its own version against the numpy stubs resolved
+# for it.  Pinning 3.10 made mypy reject numpy's `type` statements outright.
 strict = true
 warn_return_any = true
 warn_unused_ignores = true

tests/test_accessor.py

@@ -498,12 +498,118 @@ def test_imshow_auto_skips_facet_row_on_old_plotly(
     ) -> None:
         """Test that auto-assignment skips facet_row on old plotly (4th dim animates)."""
         monkeypatch.setattr(plotting, "_imshow_supports_facet_row", lambda: False)
-        fig = self.da_4d.plotly.imshow()
+        with pytest.warns(UserWarning, match=r"'year' is animated instead of faceted"):
+            fig = self.da_4d.plotly.imshow()
         # year (4th dim) falls through to animation_frame instead of facet_row
         assert len(fig.frames) == 3
         # only the facet_col (scenario) produces subplots
         assert len(fig.data) == 2
 
+    def test_imshow_auto_facet_row_warning_mentions_plotly_version(
+        self, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """Test that the fallback warning points at the plotly requirement."""
+        monkeypatch.setattr(plotting, "_imshow_supports_facet_row", lambda: False)
+        with pytest.warns(UserWarning, match=r"facet_row for imshow requires plotly>=6\.7\.0"):
+            self.da_4d.plotly.imshow()
+
+    def test_imshow_no_animation_slot_left_on_old_plotly(
+        self, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """Test the 5D case on old plotly: no slot left to absorb facet_row."""
+        monkeypatch.setattr(plotting, "_imshow_supports_facet_row", lambda: False)
+        da = xr.DataArray(
+            np.random.rand(2, 2, 2, 2, 2),
+            dims=["lat", "lon", "scenario", "year", "time"],
+        )
+        with pytest.raises(ValueError, match=r"already fills the animation slot"):
+            da.plotly.imshow()
+
+    @requires_imshow_facet_row
+    def test_imshow_facet_col_wrap_ignored_with_facet_row(self) -> None:
+        """Test that facet_col_wrap is dropped so facet_row titles survive."""
+        with pytest.warns(UserWarning, match=r"facet_col_wrap is ignored"):
+            fig = self.da_4d.plotly.imshow(facet_col_wrap=2)
+        facet_titles = {a.text for a in fig.layout.annotations if "=" in (a.text or "")}
+        assert facet_titles == {
+            "scenario=low",
+            "scenario=high",
+            "year=2020",
+            "year=2021",
+            "year=2022",
+        }
+
+    def test_imshow_facet_col_wrap_kept_without_facet_row(self) -> None:
+        """Test that facet_col_wrap still applies when there is no facet_row."""
+        fig = self.da_3d.plotly.imshow(facet_col_wrap=2)
+        assert len(fig.data) == 3
+        # Wrapping at 2 columns stacks the 3 facets over two rows of subplots.
+        domains = {tuple(fig.layout[k].domain) for k in fig.layout if k.startswith("yaxis")}
+        assert len(domains) == 2
+
+    def test_imshow_duplicate_dim_across_slots(self) -> None:
+        """Test a clear error when one dimension is asked to fill two slots.
+
+        px.imshow otherwise dies with "IndexError: pop index out of range".
+        """
+        with pytest.raises(ValueError, match=r"'scenario' is assigned to both"):
+            self.da_3d.plotly.imshow(y="lat", x="lon", facet_col="scenario", facet_row="scenario")
+
+    def test_imshow_duplicate_dim_facet_row_and_animation(self) -> None:
+        """Test the duplicate check across facet_row and animation_frame."""
+        with pytest.raises(ValueError, match=r"'year' is assigned to both"):
+            self.da_4d.plotly.imshow(
+                y="lat", x="lon", facet_col="scenario", facet_row="year", animation_frame="year"
+            )
+
+    def test_imshow_no_dimension_left_for_x(self) -> None:
+        """Test a clear error when facet/animation slots eat the heatmap axes.
+
+        px.imshow otherwise dies with "IndexError: list index out of range".
+        """
+        with pytest.raises(ValueError, match=r"needs a dimension for both 'y' and 'x'"):
+            self.da_4d.plotly.imshow(facet_col="lat", facet_row="lon", animation_frame="scenario")
+
+    def test_imshow_2d_with_both_facets_leaves_no_axes(self) -> None:
+        """Test the error when a 2D array puts both of its dims into facets."""
+        da = xr.DataArray(
+            np.random.rand(2, 3), dims=["a", "b"], coords={"a": [0, 1], "b": [0, 1, 2]}
+        )
+        with pytest.raises(ValueError, match=r"needs a dimension for both 'y' and 'x'"):
+            da.plotly.imshow(facet_col="a", facet_row="b")
+
+    @requires_imshow_facet_row
+    def test_imshow_explicit_x_y_facet_col_facet_row_4d(self) -> None:
+        """Test that naming all four slots on a 4D array builds the full grid."""
+        fig = self.da_4d.plotly.imshow(x="lon", y="lat", facet_col="scenario", facet_row="year")
+        assert len(fig.data) == 6
+        facet_titles = {a.text for a in fig.layout.annotations if "=" in (a.text or "")}
+        assert facet_titles == {
+            "scenario=low",
+            "scenario=high",
+            "year=2020",
+            "year=2021",
+            "year=2022",
+        }
+
+    @requires_imshow_facet_row
+    def test_imshow_facet_grid_places_data_in_right_subplot(self) -> None:
+        """Test that each (facet_col, facet_row) pair lands in its own subplot."""
+        values = np.zeros((2, 3, 4, 5))
+        for col in range(2):
+            for row in range(3):
+                values[col, row] = col * 10 + row
+        da = xr.DataArray(
+            values,
+            dims=["scenario", "year", "lat", "lon"],
+            coords={"scenario": ["low", "high"], "year": [2020, 2021, 2022]},
+        )
+        fig = da.plotly.imshow(x="lon", y="lat", facet_col="scenario", facet_row="year")
+        assert len(fig.data) == 6
+        # Every subplot holds exactly one constant value, and all six differ.
+        constants = {float(np.unique(trace.z)[0]) for trace in fig.data}
+        assert constants == {0.0, 1.0, 2.0, 10.0, 11.0, 12.0}
+
 
 class TestColorsParameter:
     """Tests for the unified colors parameter."""

xarray_plotly/accessor.py

@@ -360,8 +360,10 @@ def imshow(
             y: Dimension for y-axis (rows). Default: first dimension.
             facet_col: Dimension for subplot columns. Default: third dimension.
             facet_row: Dimension for subplot rows. Default: fourth dimension.
-                Requires plotly>=6.7.0; on older versions this slot is skipped
-                during auto-assignment.
+                Requires plotly>=6.7.0; on older versions an auto-assigned
+                dimension animates instead (with a warning) and an explicitly
+                named one raises `ValueError`. `facet_col_wrap` is ignored
+                when `facet_row` is set.
             animation_frame: Dimension for animation. Default: fifth dimension.
             robust: If True, use 2nd/98th percentiles for color bounds (handles outliers).
             colors: Color scale name (e.g., "Viridis", "RdBu"). See module docs.

xarray_plotly/plotting.py

@@ -31,6 +31,8 @@
 )
 
 if TYPE_CHECKING:
+    from collections.abc import Hashable
+
     import plotly.graph_objects as go
     from xarray import DataArray
 
@@ -669,6 +671,88 @@ def _imshow_supports_facet_row() -> bool:
     return "facet_row" in inspect.signature(px.imshow).parameters
 
 
+_IMSHOW_SLOTS = ("y", "x", "facet_col", "facet_row", "animation_frame")
+
+
+def _validate_imshow_slots(slots: dict[str, Hashable]) -> None:
+    """Check that imshow's slots form a usable heatmap before handing them to plotly.
+
+    Every imshow slot is a separate axis of the data, so each needs its own
+    dimension and both heatmap axes must be filled.  ``px.imshow`` does not
+    check either, and fails deep inside its own slicing with ``IndexError:
+    pop index out of range`` (a dimension used twice) or ``IndexError: list
+    index out of range`` (nothing left for y/x).
+
+    Args:
+        slots: Slot assignment from :func:`assign_slots`.
+
+    Raises:
+        ValueError: If a dimension fills two slots, or y/x is left empty.
+    """
+    seen: dict[Hashable, str] = {}
+    for slot in _IMSHOW_SLOTS:
+        dim = slots.get(slot)
+        if dim is None:
+            continue
+        if dim in seen:
+            msg = (
+                f"Dimension {dim!r} is assigned to both {seen[dim]!r} and {slot!r}. "
+                f"Each imshow slot needs its own dimension."
+            )
+            raise ValueError(msg)
+        seen[dim] = slot
+
+    missing = [slot for slot in ("y", "x") if slots.get(slot) is None]
+    if missing:
+        taken = {slot: dim for dim, slot in seen.items()}
+        msg = (
+            f"imshow needs a dimension for both 'y' and 'x', but {missing} "
+            f"came up empty; the other slots took {taken}. Free one with "
+            f"facet_col=None, facet_row=None or animation_frame=None, or reduce "
+            f"a dimension with .sel(), .isel() or .mean() before plotting."
+        )
+        raise ValueError(msg)
+
+
+def _handle_unsupported_facet_row(slots: dict[str, Hashable], *, explicit: bool) -> None:
+    """Resolve an imshow ``facet_row`` slot that the installed plotly cannot draw.
+
+    ``px.imshow`` gained ``facet_row`` in plotly 6.7.0.  On older versions an
+    explicit request is an error, while an auto-assigned dimension falls back
+    to animating (with a warning, so the missing subplot rows are not a
+    silent surprise).  If the animation slot is already taken there is nowhere
+    to fall back to, so that case raises as well.
+
+    Args:
+        slots: Slot assignment from :func:`assign_slots` (mutated in place).
+        explicit: Whether the user named the ``facet_row`` dimension.
+    """
+    import plotly
+
+    dim = slots["facet_row"]
+    msg = f"facet_row for imshow requires plotly>=6.7.0 (installed: {plotly.__version__})."
+
+    if explicit:
+        raise ValueError(msg)
+
+    if slots.get("animation_frame") is not None:
+        msg = (
+            f"{msg} Dimension {dim!r} cannot be faceted across subplot rows, and "
+            f"{slots['animation_frame']!r} already fills the animation slot. "
+            f"Upgrade plotly, or reduce a dimension with .sel(), .isel() or .mean()."
+        )
+        raise ValueError(msg)
+
+    warnings.warn(
+        f"{msg} Dimension {dim!r} is animated instead of faceted across "
+        f"subplot rows; upgrade plotly to facet it.",
+        UserWarning,
+        stacklevel=4,
+    )
+    slots["animation_frame"] = dim
+    slots["facet_row"] = None
+
+
 def imshow(
     darray: DataArray,
     *,
@@ -709,10 +793,11 @@ def imshow(
         Dimension for subplot columns. Default: third dimension.
     facet_row
         Dimension for subplot rows. Default: fourth dimension.
-        Requires plotly>=6.7.0; on older versions this slot is skipped
-        during auto-assignment (the fourth dimension animates instead).
-        Note: ``facet_col_wrap`` is ignored by plotly when ``facet_row``
-        is set.
+        Requires plotly>=6.7.0; on older versions an auto-assigned
+        dimension animates instead and a ``UserWarning`` is emitted, while
+        an explicitly named one raises ``ValueError``.
+        Note: ``facet_col_wrap`` is ignored (with a warning) when
+        ``facet_row`` is set, matching the other plot types.
     animation_frame
         Dimension for animation. Default: fifth dimension.
     robust
@@ -736,11 +821,6 @@ def imshow(
     """
     px_kwargs = resolve_colors(colors, px_kwargs)
 
-    # On plotly < 6.7.0, px.imshow has no facet_row: skip auto-assignment so
-    # dimensions fall through to animation_frame instead.
-    if facet_row is auto and not _imshow_supports_facet_row():
-        facet_row = None
-
     slots = assign_slots(
         list(darray.dims),
         "imshow",
@@ -751,14 +831,24 @@ def imshow(
         animation_frame=animation_frame,
     )
 
+    _validate_imshow_slots(slots)
+
+    if slots.get("facet_row") is not None and not _imshow_supports_facet_row():
+        _handle_unsupported_facet_row(slots, explicit=facet_row is not auto)
+
     facet_row_kwargs: dict[str, Any] = {}
     if slots.get("facet_row") is not None:
-        if not _imshow_supports_facet_row():
-            import plotly
-
-            msg = f"facet_row for imshow requires plotly>=6.7.0 (installed: {plotly.__version__})."
-            raise ValueError(msg)
         facet_row_kwargs["facet_row"] = slots["facet_row"]
+        # px.imshow honours facet_col_wrap even when facet_row is set, which
+        # builds the grid but silently drops the facet_row titles.  Every other
+        # px function ignores the wrap in that case; match them.
+        if px_kwargs.pop("facet_col_wrap", None) is not None:
+            warnings.warn(
+                "facet_col_wrap is ignored when facet_row is set; "
+                "px.imshow would otherwise drop the facet_row subplot titles.",
+                UserWarning,
+                stacklevel=3,
+            )
 
     # Transpose to: y (rows), x (cols), facet_col, facet_row, animation_frame
     transpose_order = [