Skip to content

Commit d9ef1f0

Browse files
max-sixtyclaudepre-commit-ci[bot]mathause
authored
Fix assert_equal with check_dim_order=False for mixed dimension orders (#10718)
* Fix assert_equal with check_dim_order=False for Datasets with mixed dimension orders Fixes #10704 The bug: assert_equal with check_dim_order=False was failing when comparing Datasets containing variables with different dimension orders. It would even fail when comparing a Dataset to itself. The fix: Transpose both objects to a canonical dimension order using the intersection of their dimensions. The ellipsis (...) handles any dimensions unique to either object, making the solution general and elegant. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Address review feedback: avoid sorting non-sortable dimension names - Use list(common_dims) instead of sorted(common_dims) since dimensions only need to be hashable, not sortable - Add test case for datasets with non-sortable dimension names (e.g., int and str) - Transpose both a and b to the same canonical order for consistency 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add full test coverage for maybe_transpose_dims changes - Add test for no common dimensions path - Add test for Variable type specifically (not just DataArray) - Now all code paths in maybe_transpose_dims are covered 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix DataTree handling in maybe_transpose_dims The reviewer was correct - the DataTree handling was wrong. We were only transposing b, but for consistency we need to transpose both a and b to the same canonical order, just like we do for Dataset. This fixes the issue and adds a comprehensive test case. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Trigger CI re-run for flaky Zarr test * Simplify DataTree handling in maybe_transpose_dims Use single map_over_datasets call instead of two separate calls. map_over_datasets supports tuple returns and unpacks them automatically, making the explicit lambda extraction unnecessary. Co-authored-by: Claude <noreply@anthropic.com> * Fix RTD builds by restricting test-nightly to linux/macOS platforms The `test-nightly` environment uses pandas nightly wheels from PyPI, which currently don't have win-64 builds available. This causes `pixi lock` to fail when solving for all platforms. RTD builds fail because they have no lock file cache (unlike GitHub Actions CI which caches pixi.lock). When RTD runs `pixi install -e doc`, pixi must generate the lock file from scratch, which fails on the unsolvable test-nightly/win-64 combination. This restriction can be removed once pandas nightly provides win-64 wheels again. Co-authored-by: Claude <noreply@anthropic.com> * Remove redundant numpy imports in test functions Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>
1 parent cdd7692 commit d9ef1f0

2 files changed

Lines changed: 137 additions & 12 deletions

File tree

xarray/testing/assertions.py

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -86,23 +86,33 @@ def assert_isomorphic(a: DataTree, b: DataTree):
8686

8787

8888
def maybe_transpose_dims(a, b, check_dim_order: bool):
89-
"""Helper for assert_equal/allclose/identical"""
89+
"""Helper for assert_equal/allclose/identical
90+
91+
Returns (a, b) tuple with dimensions transposed to canonical order if needed.
92+
"""
9093

9194
__tracebackhide__ = True
9295

96+
if check_dim_order:
97+
return a, b
98+
9399
def _maybe_transpose_dims(a, b):
94100
if not isinstance(a, Variable | DataArray | Dataset):
95-
return b
96-
if set(a.dims) == set(b.dims):
97-
# Ensure transpose won't fail if a dimension is missing
98-
# If this is the case, the difference will be caught by the caller
99-
return b.transpose(*a.dims)
100-
return b
101-
102-
if check_dim_order:
103-
return b
101+
return a, b
102+
103+
# Find common dimensions and transpose both to canonical order
104+
common_dims = set(a.dims) & set(b.dims)
105+
if common_dims:
106+
# Use order from the intersection, with ellipsis for any unique dims
107+
canonical_order = list(common_dims) + [...]
108+
# For Datasets, we need to transpose both to the same order
109+
# For Variable/DataArray, we could just transpose b, but for consistency
110+
# and simplicity we transpose both
111+
return a.transpose(*canonical_order), b.transpose(*canonical_order)
112+
return a, b
104113

105114
if isinstance(a, DataTree):
115+
# map_over_datasets supports tuple returns and unpacks them automatically
106116
return map_over_datasets(_maybe_transpose_dims, a, b)
107117

108118
return _maybe_transpose_dims(a, b)
@@ -140,7 +150,7 @@ def assert_equal(a, b, check_dim_order: bool = True):
140150
assert type(a) is type(b) or (
141151
isinstance(a, Coordinates) and isinstance(b, Coordinates)
142152
)
143-
b = maybe_transpose_dims(a, b, check_dim_order)
153+
a, b = maybe_transpose_dims(a, b, check_dim_order)
144154
if isinstance(a, Variable | DataArray):
145155
assert a.equals(b), formatting.diff_array_repr(a, b, "equals")
146156
elif isinstance(a, Dataset):
@@ -228,7 +238,7 @@ def assert_allclose(
228238
"""
229239
__tracebackhide__ = True
230240
assert type(a) is type(b)
231-
b = maybe_transpose_dims(a, b, check_dim_order)
241+
a, b = maybe_transpose_dims(a, b, check_dim_order)
232242

233243
equiv = functools.partial(
234244
_data_allclose_or_equiv, rtol=rtol, atol=atol, decode_bytes=decode_bytes

xarray/tests/test_assertions.py

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,26 @@ def test_assert_equal_transpose_datatree() -> None:
115115

116116
xr.testing.assert_equal(a, b, check_dim_order=False)
117117

118+
# Test with mixed dimension orders in datasets (the tricky case)
119+
ds_mixed = xr.Dataset(
120+
{
121+
"foo": xr.DataArray(np.zeros([4, 5]), dims=("a", "b")),
122+
"bar": xr.DataArray(np.ones([5, 4]), dims=("b", "a")),
123+
}
124+
)
125+
ds_mixed2 = xr.Dataset(
126+
{
127+
"foo": xr.DataArray(np.zeros([5, 4]), dims=("b", "a")),
128+
"bar": xr.DataArray(np.ones([4, 5]), dims=("a", "b")),
129+
}
130+
)
131+
132+
tree1 = xr.DataTree.from_dict({"node": ds_mixed})
133+
tree2 = xr.DataTree.from_dict({"node": ds_mixed2})
134+
135+
# Should work with check_dim_order=False
136+
xr.testing.assert_equal(tree1, tree2, check_dim_order=False)
137+
118138

119139
@pytest.mark.filterwarnings("error")
120140
@pytest.mark.parametrize(
@@ -241,6 +261,101 @@ def __array__(
241261
assert len(w) == 0
242262

243263

264+
def test_assert_equal_dataset_check_dim_order():
265+
"""Test for issue #10704 - check_dim_order=False with Datasets containing mixed dimension orders."""
266+
# Dataset with variables having different dimension orders
267+
dataset_1 = xr.Dataset(
268+
{
269+
"foo": xr.DataArray(np.zeros([4, 5]), dims=("a", "b")),
270+
"bar": xr.DataArray(np.ones([5, 4]), dims=("b", "a")),
271+
}
272+
)
273+
274+
dataset_2 = xr.Dataset(
275+
{
276+
"foo": xr.DataArray(np.zeros([5, 4]), dims=("b", "a")),
277+
"bar": xr.DataArray(np.ones([4, 5]), dims=("a", "b")),
278+
}
279+
)
280+
281+
# These should be equal when ignoring dimension order
282+
xr.testing.assert_equal(dataset_1, dataset_2, check_dim_order=False)
283+
xr.testing.assert_allclose(dataset_1, dataset_2, check_dim_order=False)
284+
285+
# Should also work when comparing dataset to itself
286+
xr.testing.assert_equal(dataset_1, dataset_1, check_dim_order=False)
287+
xr.testing.assert_allclose(dataset_1, dataset_1, check_dim_order=False)
288+
289+
# But should fail with check_dim_order=True
290+
with pytest.raises(AssertionError):
291+
xr.testing.assert_equal(dataset_1, dataset_2, check_dim_order=True)
292+
with pytest.raises(AssertionError):
293+
xr.testing.assert_allclose(dataset_1, dataset_2, check_dim_order=True)
294+
295+
# Test with non-sortable dimension names (int and str)
296+
dataset_mixed_1 = xr.Dataset(
297+
{
298+
"foo": xr.DataArray(np.zeros([4, 5]), dims=(1, "b")),
299+
"bar": xr.DataArray(np.ones([5, 4]), dims=("b", 1)),
300+
}
301+
)
302+
303+
dataset_mixed_2 = xr.Dataset(
304+
{
305+
"foo": xr.DataArray(np.zeros([5, 4]), dims=("b", 1)),
306+
"bar": xr.DataArray(np.ones([4, 5]), dims=(1, "b")),
307+
}
308+
)
309+
310+
# Should work with mixed types when ignoring dimension order
311+
xr.testing.assert_equal(dataset_mixed_1, dataset_mixed_2, check_dim_order=False)
312+
xr.testing.assert_equal(dataset_mixed_1, dataset_mixed_1, check_dim_order=False)
313+
314+
315+
def test_assert_equal_no_common_dims():
316+
"""Test assert_equal when objects have no common dimensions."""
317+
# DataArrays with completely different dimensions
318+
da1 = xr.DataArray(np.zeros([4, 5]), dims=("x", "y"))
319+
da2 = xr.DataArray(np.zeros([3, 2]), dims=("a", "b"))
320+
321+
# Should fail even with check_dim_order=False since dims are different
322+
with pytest.raises(AssertionError):
323+
xr.testing.assert_equal(da1, da2, check_dim_order=False)
324+
325+
# Datasets with no common dimensions
326+
ds1 = xr.Dataset(
327+
{
328+
"foo": xr.DataArray(np.zeros([4]), dims=("x",)),
329+
"bar": xr.DataArray(np.ones([5]), dims=("y",)),
330+
}
331+
)
332+
ds2 = xr.Dataset(
333+
{
334+
"foo": xr.DataArray(np.zeros([3]), dims=("a",)),
335+
"bar": xr.DataArray(np.ones([2]), dims=("b",)),
336+
}
337+
)
338+
339+
# Should fail since dimensions are completely different
340+
with pytest.raises(AssertionError):
341+
xr.testing.assert_equal(ds1, ds2, check_dim_order=False)
342+
343+
344+
def test_assert_equal_variable_transpose():
345+
"""Test assert_equal with transposed Variable objects."""
346+
# Variables with transposed dimensions
347+
var1 = xr.Variable(("x", "y"), np.zeros([4, 5]))
348+
var2 = xr.Variable(("y", "x"), np.zeros([5, 4]))
349+
350+
# Should fail with check_dim_order=True
351+
with pytest.raises(AssertionError):
352+
xr.testing.assert_equal(var1, var2, check_dim_order=True)
353+
354+
# Should pass with check_dim_order=False
355+
xr.testing.assert_equal(var1, var2, check_dim_order=False)
356+
xr.testing.assert_allclose(var1, var2, check_dim_order=False)
357+
358+
244359
class CustomIndex(Index):
245360
"""Custom index without equals() implementation for testing."""
246361

0 commit comments

Comments
 (0)