diff --git a/pyproject.toml b/pyproject.toml index 299232a..e43aea5 100644 --- a/pyproject.toml +++ b/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 diff --git a/tests/test_accessor.py b/tests/test_accessor.py index aa8f1a4..61bfbd6 100644 --- a/tests/test_accessor.py +++ b/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.""" diff --git a/xarray_plotly/accessor.py b/xarray_plotly/accessor.py index ec4fa50..cab0ed3 100644 --- a/xarray_plotly/accessor.py +++ b/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. diff --git a/xarray_plotly/plotting.py b/xarray_plotly/plotting.py index d52abce..360bccb 100644 --- a/xarray_plotly/plotting.py +++ b/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 = [