diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 864b344c4ab..39cca8b977b 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -154,6 +154,12 @@ Bug Fixes - Fixed a bug that caused rechunking a multi-dimensional cftime array along a subset of its dimensions to raise an error (:issue:`11567`, :pull:`11576`). By `Spencer Clark `_. +- :py:func:`align` now raises ``AlignmentError`` when one index would reorder a + dimension that another, already aligned, index shares. Previously an index + requiring no reindexing was skipped by the conflict check, so conflicting + indexes aligned silently and the mismatched index was used to combine the data + (:issue:`10714`). + By `Kayvan Zahiri `_. - :py:meth:`Dataset.diff` and :py:meth:`DataArray.diff` now raise a ``ValueError`` when ``dim`` is not an existing dimension, instead of silently returning the object unchanged. This matches the behavior of other methods such as diff --git a/xarray/structure/alignment.py b/xarray/structure/alignment.py index eebceddde97..285fcf8ffca 100644 --- a/xarray/structure/alignment.py +++ b/xarray/structure/alignment.py @@ -561,6 +561,19 @@ def _get_dim_pos_indexers( dim_pos_indexers: dict[Hashable, Any] = {} dim_index: dict[Hashable, Index] = {} + # An index that needs no reindexing is equal across all objects, so it pins + # its dimensions in place. It never produces an indexer of its own, which + # means the conflict check below would otherwise never see it and a second + # index sharing the same dimension could silently reorder it. + unchanged_dim_index: dict[Hashable, Index] = {} + for key in self.aligned_indexes: + obj_idx = matching_indexes.get(key) + if obj_idx is not None and not self.reindex[key]: + for dim in { + d for var in self.aligned_index_vars[key].values() for d in var.dims + }: + unchanged_dim_index.setdefault(dim, obj_idx) + for key, aligned_idx in self.aligned_indexes.items(): obj_idx = matching_indexes.get(key) if obj_idx is not None and self.reindex[key]: @@ -573,6 +586,19 @@ def _get_dim_pos_indexers( "wrong results returned by the `reindex_like` method of this index:\n" f"{obj_idx!r}" ) + idxer_arr = np.asarray(idxer) + reorders = not ( + idxer_arr.ndim == 1 + and np.array_equal(idxer_arr, np.arange(idxer_arr.size)) + ) + if dim in unchanged_dim_index and reorders: + raise AlignmentError( + f"cannot reindex or align along dimension {dim!r} because " + "it would reorder another index that is already aligned along " + "that dimension\n" + f"first index: {obj_idx!r}\n" + f"second index: {unchanged_dim_index[dim]!r}\n" + ) if dim in dim_pos_indexers and not np.array_equal( idxer, dim_pos_indexers[dim] ): diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index c9001c95141..01fb63ebc79 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -2731,6 +2731,23 @@ def test_align_multiple_indexes_common_dim(self) -> None: with pytest.raises(AlignmentError, match=r".*conflicting re-indexers"): align(a, c) + def test_align_multiple_indexes_common_dim_no_reindex(self) -> None: + # An index that needs no reindexing produces no re-indexer, so it used to + # escape the conflict check entirely and a second index sharing its + # dimension could silently reorder it. See GH10714. + a = Dataset(coords={"x": [1, 2, 3], "xb": ("x", [4, 5, 6])}).set_xindex("xb") + # "x" is equal in both and needs no reindexing; only "xb" conflicts + b = Dataset(coords={"x": [1, 2, 3], "xb": ("x", [4, 6, 5])}).set_xindex("xb") + + with pytest.raises(AlignmentError, match=r".*would reorder another index"): + align(a, b) + + # reordering a dimension is still fine when nothing else pins it + d = Dataset(coords={"x": [3, 1, 2], "xb": ("x", [6, 4, 5])}).set_xindex("xb") + (a2, d2) = align(a, d) + assert_identical(a2, a, check_default_indexes=False) + assert_identical(d2, a, check_default_indexes=False) + def test_align_conflicting_indexes(self) -> None: class CustomIndex(PandasIndex): ...