From ec27da4765dd11f7de6c39e903a8157b5a6f6325 Mon Sep 17 00:00:00 2001 From: dcherian Date: Sat, 15 Jul 2023 12:28:24 -0500 Subject: [PATCH 1/5] Warn instead of raising for nD index variable Avoid automatic creating of Index variable when nD variable shares name with one of its dimensions. Closes #2233 --- xarray/core/variable.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/xarray/core/variable.py b/xarray/core/variable.py index 9271d0c4dbd..7c84f6adb49 100644 --- a/xarray/core/variable.py +++ b/xarray/core/variable.py @@ -161,12 +161,14 @@ def as_variable(obj, name=None) -> Variable | IndexVariable: if name is not None and name in obj.dims: # convert the Variable into an Index if obj.ndim != 1: - raise MissingDimensionsError( + warnings.warn( f"{name!r} has more than 1-dimension and the same name as one of its " - f"dimensions {obj.dims!r}. xarray disallows such variables because they " - "conflict with the coordinates used to label dimensions." + f"dimensions {obj.dims!r}. Xarray will not automatically" + "create an Index object that would allow label-based selection.", + RuntimeWarning, ) - obj = obj.to_index_variable() + else: + obj = obj.to_index_variable() return obj From 5164d1e3d5a22ad9e3c7395092807c143e8423bc Mon Sep 17 00:00:00 2001 From: dcherian Date: Sat, 15 Jul 2023 15:14:13 -0500 Subject: [PATCH 2/5] fix tests --- xarray/core/variable.py | 2 +- xarray/tests/test_dataset.py | 6 ++++-- xarray/tests/test_variable.py | 5 +++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/xarray/core/variable.py b/xarray/core/variable.py index 7c84f6adb49..65bee3b359b 100644 --- a/xarray/core/variable.py +++ b/xarray/core/variable.py @@ -163,7 +163,7 @@ def as_variable(obj, name=None) -> Variable | IndexVariable: if obj.ndim != 1: warnings.warn( f"{name!r} has more than 1-dimension and the same name as one of its " - f"dimensions {obj.dims!r}. Xarray will not automatically" + f"dimensions {obj.dims!r}. Xarray will not automatically " "create an Index object that would allow label-based selection.", RuntimeWarning, ) diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index 01c26ad6104..578bf2b6332 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -467,13 +467,15 @@ def test_constructor(self) -> None: with pytest.raises(ValueError, match=r"conflicting sizes"): Dataset({"a": x1, "b": x2}) - with pytest.raises(ValueError, match=r"disallows such variables"): - Dataset({"a": x1, "x": z}) with pytest.raises(TypeError, match=r"tuple of form"): Dataset({"x": (1, 2, 3, 4, 5, 6, 7)}) with pytest.raises(ValueError, match=r"already exists as a scalar"): Dataset({"x": 0, "y": ("x", [1, 2, 3])}) + with pytest.warns(RuntimeWarning, match=r"will not automatically"): + actual = Dataset({"a": x1, "x": z}) + assert "x" not in actual.xindexes + # verify handling of DataArrays expected = Dataset({"x": x1, "z": z}) actual = Dataset({"z": expected["z"]}) diff --git a/xarray/tests/test_variable.py b/xarray/tests/test_variable.py index a6dffb82660..ec9ad172437 100644 --- a/xarray/tests/test_variable.py +++ b/xarray/tests/test_variable.py @@ -1247,8 +1247,9 @@ def test_as_variable(self): expected = Variable(("x", "y"), data) with pytest.raises(ValueError, match=r"without explicit dimension names"): as_variable(data, name="x") - with pytest.raises(ValueError, match=r"has more than 1-dimension"): - as_variable(expected, name="x") + with pytest.warns(RuntimeWarning, match=r"has more than 1-dimension"): + actual = as_variable(expected, name="x") + assert_identical(expected, actual) # test datetime, timedelta conversion dt = np.array([datetime(1999, 1, 1) + timedelta(days=x) for x in range(10)]) From 1f92e302d530ef4c215d6404b2641be052ef6298 Mon Sep 17 00:00:00 2001 From: dcherian Date: Sun, 16 Jul 2023 10:01:59 -0500 Subject: [PATCH 3/5] Remove warning --- xarray/core/variable.py | 14 +++----------- xarray/tests/test_dataset.py | 6 +++--- xarray/tests/test_variable.py | 7 ++++--- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/xarray/core/variable.py b/xarray/core/variable.py index 65bee3b359b..f2b05bb441f 100644 --- a/xarray/core/variable.py +++ b/xarray/core/variable.py @@ -158,17 +158,9 @@ def as_variable(obj, name=None) -> Variable | IndexVariable: f"explicit list of dimensions: {obj!r}" ) - if name is not None and name in obj.dims: - # convert the Variable into an Index - if obj.ndim != 1: - warnings.warn( - f"{name!r} has more than 1-dimension and the same name as one of its " - f"dimensions {obj.dims!r}. Xarray will not automatically " - "create an Index object that would allow label-based selection.", - RuntimeWarning, - ) - else: - obj = obj.to_index_variable() + if name is not None and name in obj.dims and obj.ndim == 1: + # automatically convert the Variable into an Index + obj = obj.to_index_variable() return obj diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index 578bf2b6332..b20cb91cdbc 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -472,9 +472,9 @@ def test_constructor(self) -> None: with pytest.raises(ValueError, match=r"already exists as a scalar"): Dataset({"x": 0, "y": ("x", [1, 2, 3])}) - with pytest.warns(RuntimeWarning, match=r"will not automatically"): - actual = Dataset({"a": x1, "x": z}) - assert "x" not in actual.xindexes + # nD coordinate variable "x" sharing name with dimension + actual = Dataset({"a": x1, "x": z}) + assert "x" not in actual.xindexes # verify handling of DataArrays expected = Dataset({"x": x1, "z": z}) diff --git a/xarray/tests/test_variable.py b/xarray/tests/test_variable.py index ec9ad172437..9b70dcb5464 100644 --- a/xarray/tests/test_variable.py +++ b/xarray/tests/test_variable.py @@ -1247,9 +1247,10 @@ def test_as_variable(self): expected = Variable(("x", "y"), data) with pytest.raises(ValueError, match=r"without explicit dimension names"): as_variable(data, name="x") - with pytest.warns(RuntimeWarning, match=r"has more than 1-dimension"): - actual = as_variable(expected, name="x") - assert_identical(expected, actual) + + # name of nD variable matches dimension name + actual = as_variable(expected, name="x") + assert_identical(expected, actual) # test datetime, timedelta conversion dt = np.array([datetime(1999, 1, 1) + timedelta(days=x) for x in range(10)]) From f059e50f45ddb52e26f824e87aba7a3d94fa573d Mon Sep 17 00:00:00 2001 From: dcherian Date: Sun, 16 Jul 2023 10:37:05 -0500 Subject: [PATCH 4/5] Add invariants check Co-authored-by: Benoit Bovy --- xarray/core/indexes.py | 2 +- xarray/testing.py | 15 +++++++-------- xarray/tests/test_dataset.py | 2 ++ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/xarray/core/indexes.py b/xarray/core/indexes.py index 9ee9bc374d4..7159178aa3c 100644 --- a/xarray/core/indexes.py +++ b/xarray/core/indexes.py @@ -1362,7 +1362,7 @@ def default_indexes( coord_names = set(coords) for name, var in coords.items(): - if name in dims: + if name in dims and var.ndim == 1: index, index_vars = create_default_index_implicit(var, coords) if set(index_vars) <= coord_names: indexes.update({k: index for k in index_vars}) diff --git a/xarray/testing.py b/xarray/testing.py index b6a88135ee1..80c73430312 100644 --- a/xarray/testing.py +++ b/xarray/testing.py @@ -364,14 +364,13 @@ def _assert_dataset_invariants(ds: Dataset, check_default_indexes: bool): assert all( ds._dims[k] == v.sizes[k] for v in ds._variables.values() for k in v.sizes ), (ds._dims, {k: v.sizes for k, v in ds._variables.items()}) - assert all( - isinstance(v, IndexVariable) - for (k, v) in ds._variables.items() - if v.dims == (k,) - ), {k: type(v) for k, v in ds._variables.items() if v.dims == (k,)} - assert all(v.dims == (k,) for (k, v) in ds._variables.items() if k in ds._dims), { - k: v.dims for k, v in ds._variables.items() if k in ds._dims - } + + if check_default_indexes: + assert all( + isinstance(v, IndexVariable) + for (k, v) in ds._variables.items() + if v.dims == (k,) + ), {k: type(v) for k, v in ds._variables.items() if v.dims == (k,)} if ds._indexes is not None: _assert_indexes_invariants_checks( diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index b20cb91cdbc..f7f91d0e134 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -35,6 +35,7 @@ from xarray.core.indexes import Index, PandasIndex from xarray.core.pycompat import array_type, integer_types from xarray.core.utils import is_scalar +from xarray.testing import _assert_internal_invariants from xarray.tests import ( DuckArrayWrapper, InaccessibleArray, @@ -475,6 +476,7 @@ def test_constructor(self) -> None: # nD coordinate variable "x" sharing name with dimension actual = Dataset({"a": x1, "x": z}) assert "x" not in actual.xindexes + _assert_internal_invariants(actual, check_default_indexes=True) # verify handling of DataArrays expected = Dataset({"x": x1, "z": z}) From ba95097a21019ce375f0dcd43c556ad30557aee5 Mon Sep 17 00:00:00 2001 From: dcherian Date: Sun, 16 Jul 2023 10:42:38 -0500 Subject: [PATCH 5/5] Add whats-new --- doc/whats-new.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 3740a5a44f1..a23883338b0 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -21,6 +21,10 @@ v2023.07.1 (unreleased) New Features ~~~~~~~~~~~~ +- Allow creating Xarray objects where a multidimensional variable shares its name + with a dimension. Examples include output from finite volume models like FVCOM. + (:issue:`2233`, :pull:`7989`) + By `Deepak Cherian `_ and `Benoit Bovy `_. Breaking changes