Conversation
📝 WalkthroughWalkthroughDataset-level plotting support is added to xarray_plotly through a new DatasetPlotlyAccessor class. The xpx() function is updated to accept either DataArray or Dataset inputs and dispatch to the corresponding accessor type using overload typing. Comprehensive test coverage validates both accessor implementations. Changes
Sequence DiagramsequenceDiagram
participant User
participant xpx as xpx() Function
participant DAcc as DataArrayPlotlyAccessor
participant DSAcc as DatasetPlotlyAccessor
participant Plotting as Plotting Utilities
User->>xpx: xpx(dataarray)
xpx->>DAcc: Create accessor
DAcc-->>User: Return accessor
User->>DAcc: .line()
DAcc->>Plotting: delegate to plotting.line()
Plotting-->>DAcc: Return Figure
DAcc-->>User: Return Figure
User->>xpx: xpx(dataset)
xpx->>DSAcc: Create accessor
DSAcc-->>User: Return accessor
User->>DSAcc: .line(var=None)
DSAcc->>DSAcc: _get_dataarray() converts to DataArray
DSAcc->>Plotting: delegate to plotting.line()
Plotting-->>DSAcc: Return Figure
DSAcc-->>User: Return Figure
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/test_accessor.py (2)
37-61: Tests verify type but not actual equivalence.The test names suggest verifying equivalence between
xpx(da).line()andda.plotly.line(), but only the return type is checked. Consider adding assertions that compare trace counts or data to ensure both paths produce identical figures.💡 Optional enhancement
def test_xpx_dataarray_equivalent_to_accessor(self) -> None: """Test that xpx(da).line() works the same as da.plotly.line().""" da = xr.DataArray( np.random.rand(10, 3), dims=["time", "city"], coords={"time": np.arange(10), "city": ["A", "B", "C"]}, name="test", ) fig1 = xpx(da).line() fig2 = da.plotly.line() assert isinstance(fig1, go.Figure) assert isinstance(fig2, go.Figure) + assert len(fig1.data) == len(fig2.data)
235-294: Good test coverage for DatasetPlotlyAccessor.The test class validates accessor availability and basic plotting functionality. Consider adding a test for error handling when an invalid variable name is passed to
var.💡 Optional: Add error handling test
def test_line_invalid_variable_raises(self) -> None: """Test that invalid variable name raises KeyError.""" with pytest.raises(KeyError): self.ds.plotly.line(var="nonexistent")
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/test_accessor.pyxarray_plotly/__init__.pyxarray_plotly/accessor.py
🧰 Additional context used
🧬 Code graph analysis (1)
xarray_plotly/__init__.py (1)
xarray_plotly/accessor.py (1)
DatasetPlotlyAccessor(278-521)
🔇 Additional comments (9)
tests/test_accessor.py (1)
18-35: LGTM! Good coverage for xpx function dispatch.The tests correctly verify that
xpx()returns the appropriate accessor type based on input (DataArray vs Dataset) and validates the expected method availability, including the intentional absence ofimshowon the Dataset accessor.xarray_plotly/__init__.py (3)
67-72: LGTM! Well-structured overloads for type-safe dispatch.The overload definitions correctly specify the return types for DataArray and Dataset inputs, enabling proper IDE code completion and type checking.
75-101: LGTM! Clean implementation with correct dispatch order.The implementation correctly checks for
Datasetfirst before falling back toDataArrayPlotlyAccessor. The type hints and overloads provide adequate guidance to users.
106-108: LGTM! Accessor registration follows xarray conventions.Both DataArray and Dataset accessors are correctly registered under the same
"plotly"namespace, providing a consistent API surface.xarray_plotly/accessor.py (5)
278-317: LGTM! Well-documented class with clear examples.The class docstring clearly explains the variable plotting behavior and provides useful examples showing both all-variable and single-variable plotting patterns.
319-323: LGTM! Simple and effective helper method.The method correctly handles both cases: converting the entire Dataset to a DataArray with a "variable" dimension, or selecting a single variable. Invalid variable names will naturally raise a
KeyErrorfrom xarray, which is appropriate behavior.
325-365: LGTM! Consistent API with DataArrayPlotlyAccessor.The method signature mirrors the DataArray version with the addition of
varas a positional parameter, enabling clean usage patterns likeds.plotly.line("temp").
367-521: LGTM! All plotting methods follow consistent patterns.The
bar,area,scatter, andboxmethods correctly mirror their DataArray counterparts with consistent parameter signatures and defaults. The delegation to the sharedplottingmodule ensures consistent behavior.
310-310: LGTM! Correct method exposure.The
__all__list appropriately excludesimshow, which makes sense sinceDataset.to_array()produces a higher-dimensional array that isn't suitable for heatmap visualization without additional dimension handling.
1 participant
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.