From e00ac3fdc5a6e259397bf3193a4c9887a70343cd Mon Sep 17 00:00:00 2001 From: dcherian Date: Thu, 28 May 2020 09:38:20 -0600 Subject: [PATCH 01/16] Convert Variable to IndexVariable when renamed to existing dim name --- xarray/core/dataset.py | 11 +++++++++++ xarray/tests/test_dataset.py | 8 ++++++++ 2 files changed, 19 insertions(+) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 7edc2fab067..94d12eb9bc7 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -3027,6 +3027,17 @@ def _rename_all(self, name_dict, dims_dict): variables, coord_names = self._rename_vars(name_dict, dims_dict) dims = self._rename_dims(dims_dict) indexes = self._rename_indexes(name_dict, dims.keys()) + + # Variable could be renamed to an existing dimension name + # in this case, convert to IndexVariable and set indexes + # GH4107 + for name in coord_names: + indexvar = variables.pop(name).to_index_variable() + variables[name] = indexvar + if indexes is None: + indexes = dict() + indexes[name] = indexvar.to_index() + return variables, coord_names, dims, indexes def rename( diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index bd1938455b1..7c7661a799d 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -2638,6 +2638,14 @@ def test_rename_vars(self): with pytest.raises(ValueError): original.rename_vars(names_dict_bad) + def test_rename_to_dim_name(self): + # regression test for GH4107 + coord_1 = xr.DataArray([1, 2], dims=["coord_1"], attrs={"attrs": True}) + ds = xr.Dataset() + ds["a"] = xr.DataArray([1, 0], [coord_1]) + obj = ds.reset_index("coord_1").rename({"coord_1_": "coord_1"}) + assert_equal(ds, obj) + def test_rename_multiindex(self): mindex = pd.MultiIndex.from_tuples( [([1, 2]), ([3, 4])], names=["level0", "level1"] From c264e42e25b74799211e55fe074e5e314908d117 Mon Sep 17 00:00:00 2001 From: dcherian Date: Thu, 28 May 2020 09:58:19 -0600 Subject: [PATCH 02/16] Add funny test --- xarray/tests/test_dataset.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index 7c7661a799d..4dcf58edc0b 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -2646,6 +2646,13 @@ def test_rename_to_dim_name(self): obj = ds.reset_index("coord_1").rename({"coord_1_": "coord_1"}) assert_equal(ds, obj) + newds = ( + ds.reset_index("coord_1") + .reset_coords("coord_1_") + .rename({"coord_1_": "coord_1"}) + ) + assert "coord_1" not in newds.data_vars + def test_rename_multiindex(self): mindex = pd.MultiIndex.from_tuples( [([1, 2]), ([3, 4])], names=["level0", "level1"] From 62ab886d07aa3e79d4f0138b708982620d95f801 Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 14 Jul 2020 08:46:16 -0600 Subject: [PATCH 03/16] minor fix --- xarray/core/dataset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 94d12eb9bc7..1def942ca0f 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -3031,7 +3031,7 @@ def _rename_all(self, name_dict, dims_dict): # Variable could be renamed to an existing dimension name # in this case, convert to IndexVariable and set indexes # GH4107 - for name in coord_names: + for name in set(coord_names) & set(self.dims): indexvar = variables.pop(name).to_index_variable() variables[name] = indexvar if indexes is None: From 9cce82e0f4d07f40aa3c29b6c55dc3c1d632555b Mon Sep 17 00:00:00 2001 From: dcherian Date: Sun, 16 Aug 2020 14:17:06 -0600 Subject: [PATCH 04/16] fix bad dataset creation --- xarray/core/dataset.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 1def942ca0f..d42b04fc52d 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -3031,12 +3031,13 @@ def _rename_all(self, name_dict, dims_dict): # Variable could be renamed to an existing dimension name # in this case, convert to IndexVariable and set indexes # GH4107 - for name in set(coord_names) & set(self.dims): + for name in set(self.dims): indexvar = variables.pop(name).to_index_variable() variables[name] = indexvar if indexes is None: indexes = dict() indexes[name] = indexvar.to_index() + coord_names.add(name) return variables, coord_names, dims, indexes From cfcbfc4badb1fd925cc3618c23a8bcd45f0240a1 Mon Sep 17 00:00:00 2001 From: dcherian Date: Sun, 16 Aug 2020 14:19:33 -0600 Subject: [PATCH 05/16] Add a test --- xarray/tests/test_dataset.py | 1 + 1 file changed, 1 insertion(+) diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index 4dcf58edc0b..6cbde82085d 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -2652,6 +2652,7 @@ def test_rename_to_dim_name(self): .rename({"coord_1_": "coord_1"}) ) assert "coord_1" not in newds.data_vars + assert_equal(newds, obj) def test_rename_multiindex(self): mindex = pd.MultiIndex.from_tuples( From bbd24d57b4a04d520d21ed4bbe03609dcaf2a286 Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 24 Nov 2020 08:10:29 -0700 Subject: [PATCH 06/16] bugfix --- xarray/core/dataset.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index d42b04fc52d..f8bc5fa1b06 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -3032,12 +3032,13 @@ def _rename_all(self, name_dict, dims_dict): # in this case, convert to IndexVariable and set indexes # GH4107 for name in set(self.dims): - indexvar = variables.pop(name).to_index_variable() - variables[name] = indexvar - if indexes is None: - indexes = dict() - indexes[name] = indexvar.to_index() - coord_names.add(name) + if name in variables: + indexvar = variables.pop(name).to_index_variable() + variables[name] = indexvar + if indexes is None: + indexes = dict() + indexes[name] = indexvar.to_index() + coord_names.add(name) return variables, coord_names, dims, indexes From 13ca72740f3e409e8433007debde612c85907989 Mon Sep 17 00:00:00 2001 From: dcherian Date: Sat, 16 Jan 2021 18:17:04 -0700 Subject: [PATCH 07/16] Fix rename_dims --- xarray/core/dataset.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index f8bc5fa1b06..4b17509a977 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -3040,6 +3040,15 @@ def _rename_all(self, name_dict, dims_dict): indexes[name] = indexvar.to_index() coord_names.add(name) + # rename_dims was called to rename an indexed dimension + # the new renamed dimension is unindexed + # So we need to remove the old variable form indexes + # and convert to a base variable + for name in dims_dict: + if name in indexes: + variables[name] = variables[name].to_base_variable() + del indexes[name] + return variables, coord_names, dims, indexes def rename( From 6d66756e333df4bcdaf1fda775a2e875cc712536 Mon Sep 17 00:00:00 2001 From: dcherian Date: Sat, 16 Jan 2021 18:20:24 -0700 Subject: [PATCH 08/16] Another bugfix --- xarray/core/dataset.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 4b17509a977..f108321f087 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -3042,9 +3042,11 @@ def _rename_all(self, name_dict, dims_dict): # rename_dims was called to rename an indexed dimension # the new renamed dimension is unindexed - # So we need to remove the old variable form indexes + # So we need to remove the old variable from indexes # and convert to a base variable - for name in dims_dict: + for name, newname in dims_dict.items(): + if name == newname: + continue if name in indexes: variables[name] = variables[name].to_base_variable() del indexes[name] From d308f9ea798a7d0ebe341b5ed6095cdad8bd67f7 Mon Sep 17 00:00:00 2001 From: dcherian Date: Sat, 16 Jan 2021 18:24:36 -0700 Subject: [PATCH 09/16] cleanup --- xarray/core/dataset.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index f108321f087..14daab7d2b4 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -3033,11 +3033,10 @@ def _rename_all(self, name_dict, dims_dict): # GH4107 for name in set(self.dims): if name in variables: - indexvar = variables.pop(name).to_index_variable() - variables[name] = indexvar + variables[name] = variables[name].to_index_variable() if indexes is None: indexes = dict() - indexes[name] = indexvar.to_index() + indexes[name] = variables[name].to_index() coord_names.add(name) # rename_dims was called to rename an indexed dimension From 945533b118a82434c69acb888991559a8748f220 Mon Sep 17 00:00:00 2001 From: dcherian Date: Sat, 16 Jan 2021 18:33:42 -0700 Subject: [PATCH 10/16] Another bugfix --- xarray/core/dataset.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 14daab7d2b4..f1b4fc0d5ba 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -3043,12 +3043,13 @@ def _rename_all(self, name_dict, dims_dict): # the new renamed dimension is unindexed # So we need to remove the old variable from indexes # and convert to a base variable - for name, newname in dims_dict.items(): - if name == newname: - continue - if name in indexes: - variables[name] = variables[name].to_base_variable() - del indexes[name] + if indexes is not None: + for name, newname in dims_dict.items(): + if name == newname: + continue + if name in indexes: + variables[name] = variables[name].to_base_variable() + del indexes[name] return variables, coord_names, dims, indexes From 20e090f5c7440c6994346db34524b244c8978e52 Mon Sep 17 00:00:00 2001 From: dcherian Date: Sat, 16 Jan 2021 18:37:39 -0700 Subject: [PATCH 11/16] [skip-ci] add whats-new --- doc/whats-new.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 88994a5bfc0..5004f51f315 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -80,6 +80,8 @@ Bug fixes - Expand user directory paths (e.g. ``~/``) in :py:func:`open_mfdataset` and :py:meth:`Dataset.to_zarr` (:issue:`4783`, :pull:`4795`). By `Julien Seguinot `_. +- Convert to IndexVariable or Variable during renaming as appropriate. (:issue:`4107`, :issue:`4417`, :pull:`4108`) + By `Deepak Cherian `_. Documentation ~~~~~~~~~~~~~ From 4c031b65ab6c4648cecab4dc53fa26b513c03ce2 Mon Sep 17 00:00:00 2001 From: dcherian Date: Fri, 12 Feb 2021 07:29:46 -0700 Subject: [PATCH 12/16] cleanup --- xarray/core/dataset.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 4346d498731..e1d7ace425f 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -2995,7 +2995,7 @@ def _rename_dims(self, name_dict): def _rename_indexes(self, name_dict, dims_set): if self._indexes is None: - return None + return {} indexes = {} for k, v in self.indexes.items(): new_name = name_dict.get(k, k) @@ -3017,23 +3017,17 @@ def _rename_all(self, name_dict, dims_dict): # Variable could be renamed to an existing dimension name # in this case, convert to IndexVariable and set indexes # GH4107 - for name in set(self.dims): - if name in variables: - variables[name] = variables[name].to_index_variable() - if indexes is None: - indexes = dict() - indexes[name] = variables[name].to_index() - coord_names.add(name) + for name in set(self.dims) & set(variables) - set(indexes): + variables[name] = variables[name].to_index_variable() + indexes[name] = variables[name].to_index() + coord_names.add(name) # rename_dims was called to rename an indexed dimension # the new renamed dimension is unindexed - # So we need to remove the old variable from indexes - # and convert to a base variable - if indexes is not None: + # remove the old variable from indexes and convert to a base variable + if indexes: for name, newname in dims_dict.items(): - if name == newname: - continue - if name in indexes: + if name != newname and name in indexes: variables[name] = variables[name].to_base_variable() del indexes[name] From fc9be4e41b2a719b1f0b708e0e43d6bfa204de8b Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Fri, 12 Feb 2021 07:35:59 -0700 Subject: [PATCH 13/16] Update xarray/core/dataset.py --- xarray/core/dataset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index e1d7ace425f..13fa933ad0d 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -2995,7 +2995,7 @@ def _rename_dims(self, name_dict): def _rename_indexes(self, name_dict, dims_set): if self._indexes is None: - return {} + return None indexes = {} for k, v in self.indexes.items(): new_name = name_dict.get(k, k) From fdec0ae0af93649d5d1bace6fe575b02ac53a611 Mon Sep 17 00:00:00 2001 From: dcherian Date: Mon, 8 Mar 2021 08:22:35 -0700 Subject: [PATCH 14/16] Fix whats-new --- doc/whats-new.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index c4c98e7eb57..e1753209a88 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -40,6 +40,8 @@ Bug fixes ~~~~~~~~~ - Don't allow passing ``axis`` to :py:meth:`Dataset.reduce` methods (:issue:`3510`, :pull:`4940`). By `Justus Magin `_. +- Convert to IndexVariable or Variable during renaming as appropriate. (:issue:`4107`, :issue:`4417`, :pull:`4108`) + By `Deepak Cherian `_. Documentation ~~~~~~~~~~~~~ @@ -207,8 +209,6 @@ Bug fixes - Expand user directory paths (e.g. ``~/``) in :py:func:`open_mfdataset` and :py:meth:`Dataset.to_zarr` (:issue:`4783`, :pull:`4795`). By `Julien Seguinot `_. -- Convert to IndexVariable or Variable during renaming as appropriate. (:issue:`4107`, :issue:`4417`, :pull:`4108`) - By `Deepak Cherian `_. - Add :py:meth:`Dataset.drop_isel` and :py:meth:`DataArray.drop_isel` (:issue:`4658`, :pull:`4819`). By `Daniel Mesejo `_. - Ensure that :py:meth:`Dataset.interp` raises ``ValueError`` when interpolating outside coordinate range and ``bounds_error=True`` (:issue:`4854`, :pull:`4855`). - Raise DeprecationWarning when trying to typecast a tuple containing a :py:class:`DataArray`. From 686ab28f60a888910319bf1520f1d235ab6d6e0d Mon Sep 17 00:00:00 2001 From: dcherian Date: Mon, 8 Mar 2021 08:26:47 -0700 Subject: [PATCH 15/16] fix tests --- xarray/core/dataset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 4479a487d8c..b42c8ec7960 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -3006,7 +3006,7 @@ def _rename_dims(self, name_dict): def _rename_indexes(self, name_dict, dims_set): if self._indexes is None: - return None + return {} indexes = {} for k, v in self.indexes.items(): new_name = name_dict.get(k, k) From e6990f71c1fc5383a493ddefc984fd9bc9fdcc0d Mon Sep 17 00:00:00 2001 From: dcherian Date: Mon, 8 Mar 2021 08:29:19 -0700 Subject: [PATCH 16/16] [skip-ci] fix whats-new --- doc/whats-new.rst | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index e1753209a88..76cf12b540f 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -209,8 +209,6 @@ Bug fixes - Expand user directory paths (e.g. ``~/``) in :py:func:`open_mfdataset` and :py:meth:`Dataset.to_zarr` (:issue:`4783`, :pull:`4795`). By `Julien Seguinot `_. -- Add :py:meth:`Dataset.drop_isel` and :py:meth:`DataArray.drop_isel` (:issue:`4658`, :pull:`4819`). By `Daniel Mesejo `_. -- Ensure that :py:meth:`Dataset.interp` raises ``ValueError`` when interpolating outside coordinate range and ``bounds_error=True`` (:issue:`4854`, :pull:`4855`). - Raise DeprecationWarning when trying to typecast a tuple containing a :py:class:`DataArray`. User now prompted to first call `.data` on it (:issue:`4483`). By `Chun Ho Chow `_.