From fe7f10089a825e7b0cb8a32964ab68e333c243ee Mon Sep 17 00:00:00 2001 From: dcherian Date: Mon, 16 Mar 2020 18:24:34 -0600 Subject: [PATCH 1/8] Raise error when assigning IndexVariable.values, IndexVariable.data Fixes #3470 --- xarray/core/variable.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/xarray/core/variable.py b/xarray/core/variable.py index 435edb6f014..7e7ae0d1058 100644 --- a/xarray/core/variable.py +++ b/xarray/core/variable.py @@ -2061,9 +2061,17 @@ def load(self): # https://github.com/python/mypy/issues/1465 @Variable.data.setter # type: ignore def data(self, data): - Variable.data.fset(self, data) - if not isinstance(self._data, PandasIndexAdapter): - self._data = PandasIndexAdapter(self._data) + raise ValueError( + f"Cannot assign to the .data attribute of dimension coordinate a.k.a IndexVariable {self.name!r}. " + f"Please use DataArray.assign_coords, Dataset.assign_coords or Dataset.assign as appropriate." + ) + + @Variable.values.setter # type: ignore + def values(self, values): + raise ValueError( + f"Cannot assign to the .values attribute of dimension coordinate a.k.a IndexVariable {self.name!r}. " + f"Please use DataArray.assign_coords, Dataset.assign_coords or Dataset.assign as appropriate." + ) def chunk(self, chunks=None, name=None, lock=False): # Dummy - do not chunk. This method is invoked e.g. by Dataset.chunk() From 997e092cad5c1feeeb805842cea8ead9ea01e8f8 Mon Sep 17 00:00:00 2001 From: dcherian Date: Mon, 16 Mar 2020 20:19:45 -0600 Subject: [PATCH 2/8] fix existing tests --- xarray/tests/test_variable.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/xarray/tests/test_variable.py b/xarray/tests/test_variable.py index c600f7a77d0..3509c955fcb 100644 --- a/xarray/tests/test_variable.py +++ b/xarray/tests/test_variable.py @@ -530,8 +530,7 @@ def test_copy_index_with_data(self): orig = IndexVariable("x", np.arange(5)) new_data = np.arange(5, 10) actual = orig.copy(data=new_data) - expected = orig.copy() - expected.data = new_data + expected = IndexVariable("x", np.arange(5, 10)) assert_identical(expected, actual) def test_copy_index_with_data_errors(self): From e7cf4bbd56de402fbe7de7ede9220735eb55332a Mon Sep 17 00:00:00 2001 From: dcherian Date: Mon, 16 Mar 2020 20:23:09 -0600 Subject: [PATCH 3/8] Add new test --- xarray/tests/test_variable.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/xarray/tests/test_variable.py b/xarray/tests/test_variable.py index 3509c955fcb..fced79fdde4 100644 --- a/xarray/tests/test_variable.py +++ b/xarray/tests/test_variable.py @@ -538,6 +538,10 @@ def test_copy_index_with_data_errors(self): new_data = np.arange(5, 20) with raises_regex(ValueError, "must match shape of object"): orig.copy(data=new_data) + with raises_regex(ValueError, "Cannot assign to the .data"): + orig.data = new_data + with raises_regex(ValueError, "Cannot assign to the .values"): + orig.values = new_data def test_replace(self): var = Variable(("x", "y"), [[1.5, 2.0], [3.1, 4.3]], {"foo": "bar"}) From 24de084d3004d2f95a2daf6b3070602ef1d31977 Mon Sep 17 00:00:00 2001 From: dcherian Date: Mon, 16 Mar 2020 20:25:45 -0600 Subject: [PATCH 4/8] whats-new --- doc/whats-new.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 80309dc4673..28844187195 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -22,6 +22,11 @@ v0.15.1 (unreleased) Breaking changes ~~~~~~~~~~~~~~~~ +- Raise an error when assigning to the ``.values`` or ``.data`` attribute of + dimension coordinates i.e. ``IndexVariable`` objects. This has been broken since + v0.13.0. (:issue:`3470`) + By `Deepak Cherian `_ + New Features ~~~~~~~~~~~~ From ad8966a7b48acea2676c24825b92ff9e0de8d8d3 Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 17 Mar 2020 07:17:02 -0600 Subject: [PATCH 5/8] Fix more existing tests --- xarray/tests/test_accessor_dt.py | 4 ++-- xarray/tests/test_dask.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/xarray/tests/test_accessor_dt.py b/xarray/tests/test_accessor_dt.py index 1a8a2732eeb..3339f5b4af5 100644 --- a/xarray/tests/test_accessor_dt.py +++ b/xarray/tests/test_accessor_dt.py @@ -80,7 +80,7 @@ def test_strftime(self): def test_not_datetime_type(self): nontime_data = self.data.copy() int_data = np.arange(len(self.data.time)).astype("int8") - nontime_data["time"].values = int_data + nontime_data = nontime_data.assign_coords(time=int_data) with raises_regex(TypeError, "dt"): nontime_data.time.dt @@ -213,7 +213,7 @@ def setup(self): def test_not_datetime_type(self): nontime_data = self.data.copy() int_data = np.arange(len(self.data.time)).astype("int8") - nontime_data["time"].values = int_data + nontime_data = nontime_data.assign_coords(time=int_data) with raises_regex(TypeError, "dt"): nontime_data.time.dt diff --git a/xarray/tests/test_dask.py b/xarray/tests/test_dask.py index 8fb54c4ee84..3aa49f3e991 100644 --- a/xarray/tests/test_dask.py +++ b/xarray/tests/test_dask.py @@ -1274,7 +1274,7 @@ def test_token_changes_when_data_changes(obj): assert t3 != t2 # Change IndexVariable - obj.coords["x"] *= 2 + obj = obj.assign_coords(x=obj.x * 2) with raise_if_dask_computes(): t4 = dask.base.tokenize(obj) assert t4 != t3 From 0722b424b547a362b5f4678483d0a42812d611ee Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Sat, 21 Mar 2020 19:10:37 +0000 Subject: [PATCH 6/8] Update doc/whats-new.rst --- doc/whats-new.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 28844187195..8188715abb8 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -24,7 +24,7 @@ Breaking changes - Raise an error when assigning to the ``.values`` or ``.data`` attribute of dimension coordinates i.e. ``IndexVariable`` objects. This has been broken since - v0.13.0. (:issue:`3470`) + v0.12.0. (:issue:`3470`, :pull:`3862`) By `Deepak Cherian `_ New Features From 104620a7c9f6719600c70a6f10f88581967d907f Mon Sep 17 00:00:00 2001 From: dcherian Date: Sun, 22 Mar 2020 16:15:42 -0600 Subject: [PATCH 7/8] fix docs --- doc/plotting.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/plotting.rst b/doc/plotting.rst index ea9816780a7..f3d9c0213de 100644 --- a/doc/plotting.rst +++ b/doc/plotting.rst @@ -657,7 +657,7 @@ Additionally, the boolean kwarg ``add_guide`` can be used to prevent the display .. ipython:: python - ds.w.values = [1, 2, 3, 5] + ds = ds.assign(w=[1, 2, 3, 5]) @savefig ds_discrete_legend_hue_scatter.png ds.plot.scatter(x='A', y='B', hue='w', hue_style='discrete') From 705c7ebb1851b3761e69580b1c4cab8e5551eaaa Mon Sep 17 00:00:00 2001 From: dcherian Date: Sun, 22 Mar 2020 16:17:22 -0600 Subject: [PATCH 8/8] update whats-new --- doc/whats-new.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 8188715abb8..c95da138e7d 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -24,7 +24,8 @@ Breaking changes - Raise an error when assigning to the ``.values`` or ``.data`` attribute of dimension coordinates i.e. ``IndexVariable`` objects. This has been broken since - v0.12.0. (:issue:`3470`, :pull:`3862`) + v0.12.0. Please use :py:meth:`DataArray.assign_coords` or :py:meth:`Dataset.assign_coords` + instead. (:issue:`3470`, :pull:`3862`) By `Deepak Cherian `_ New Features