From 436dc3affa44515dae3826b9509f04e38199d18f Mon Sep 17 00:00:00 2001 From: Gregory Gundersen Date: Mon, 19 Aug 2019 23:36:39 +0100 Subject: [PATCH 1/6] Amended docs for new API and deprecation warnings; deprecated dropping DataArrayCoordinates (#2910). --- doc/indexing.rst | 2 +- doc/whats-new.rst | 4 +++- xarray/core/dataset.py | 10 ++++++++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/doc/indexing.rst b/doc/indexing.rst index 4c5b93db0b4..b67b46c8a42 100644 --- a/doc/indexing.rst +++ b/doc/indexing.rst @@ -238,7 +238,7 @@ index labels along a dimension dropped: .. ipython:: python :okwarning: - ds.drop(['IN', 'IL'], dim='space') + ds.drop(space=['IN', 'IL']) ``drop`` is both a ``Dataset`` and ``DataArray`` method. diff --git a/doc/whats-new.rst b/doc/whats-new.rst index a127cef84f9..17382e60789 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -66,7 +66,9 @@ Enhancements By `David Brochart `_. - :py:meth:`~xarray.Dataset.drop` now supports keyword arguments; dropping index - labels by specifying both ``dim`` and ``labels`` is deprecated (:issue:`2910`). + labels by using both ``dim`` and ``labels`` or using a + :py:class:`~xarray.core.coordinates.DataArrayCoordinates` object are + deprecated (:issue:`2910`). By `Gregory Gundersen `_. - Added examples of :py:meth:`Dataset.set_index` and diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 6a606fd0c31..aa3988b61fb 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -3548,8 +3548,7 @@ def drop( # noqa: F811 if errors not in ["raise", "ignore"]: raise ValueError('errors must be either "raise" or "ignore"') - labels_are_coords = isinstance(labels, DataArrayCoordinates) - if labels_kwargs or (utils.is_dict_like(labels) and not labels_are_coords): + if labels_kwargs or isinstance(labels, dict): labels_kwargs = utils.either_dict_or_kwargs(labels, labels_kwargs, "drop") if dim is not None: raise ValueError("cannot specify dim and dict-like arguments.") @@ -3558,6 +3557,13 @@ def drop( # noqa: F811 ds = ds._drop_labels(labels, dim, errors=errors) return ds elif dim is None: + if isinstance(labels, DataArrayCoordinates): + warnings.warn( + "dropping coordinates using a DataArrayCoordinates object " + "is deprecated; use drop_vars or a list of coordinates", + DeprecationWarning, + stacklevel=2, + ) if isinstance(labels, str) or not isinstance(labels, Iterable): labels = {labels} else: From d4b532d97fac2ce8fa896b9327d595289d1d4211 Mon Sep 17 00:00:00 2001 From: Gregory Gundersen Date: Tue, 20 Aug 2019 07:20:35 +0100 Subject: [PATCH 2/6] Removed okwarning now that docs have non-deprecated API. --- doc/indexing.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/indexing.rst b/doc/indexing.rst index b67b46c8a42..58700d7b572 100644 --- a/doc/indexing.rst +++ b/doc/indexing.rst @@ -236,7 +236,6 @@ The :py:meth:`~xarray.Dataset.drop` method returns a new object with the listed index labels along a dimension dropped: .. ipython:: python - :okwarning: ds.drop(space=['IN', 'IL']) From 87c84f73faaf584e080f751b2c87ff887874dfbe Mon Sep 17 00:00:00 2001 From: Gregory Gundersen Date: Tue, 20 Aug 2019 22:04:21 +0100 Subject: [PATCH 3/6] More clear warnings and more tests. --- xarray/core/dataset.py | 22 ++++++++++++---------- xarray/tests/test_dataset.py | 10 +++++++++- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index aa3988b61fb..e5af6c5204a 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -54,7 +54,6 @@ ) from .coordinates import ( DatasetCoordinates, - DataArrayCoordinates, LevelCoordinatesSource, assert_coordinate_consistent, remap_label_indexers, @@ -77,6 +76,8 @@ either_dict_or_kwargs, hashable, maybe_wrap_array, + is_dict_like, + is_list_like, ) from .variable import IndexVariable, Variable, as_variable, broadcast_variables from ..plot.dataset_plot import _Dataset_PlotMethods @@ -3548,8 +3549,16 @@ def drop( # noqa: F811 if errors not in ["raise", "ignore"]: raise ValueError('errors must be either "raise" or "ignore"') + if is_dict_like(labels) and not isinstance(labels, dict): + warnings.warn( + "dropping coordinates using key values of dict-like labels " + "is deprecated; use drop_vars or a list of coordinates.", + FutureWarning, + stacklevel=2, + ) + if labels_kwargs or isinstance(labels, dict): - labels_kwargs = utils.either_dict_or_kwargs(labels, labels_kwargs, "drop") + labels_kwargs = either_dict_or_kwargs(labels, labels_kwargs, "drop") if dim is not None: raise ValueError("cannot specify dim and dict-like arguments.") ds = self @@ -3557,20 +3566,13 @@ def drop( # noqa: F811 ds = ds._drop_labels(labels, dim, errors=errors) return ds elif dim is None: - if isinstance(labels, DataArrayCoordinates): - warnings.warn( - "dropping coordinates using a DataArrayCoordinates object " - "is deprecated; use drop_vars or a list of coordinates", - DeprecationWarning, - stacklevel=2, - ) if isinstance(labels, str) or not isinstance(labels, Iterable): labels = {labels} else: labels = set(labels) return self._drop_vars(labels, errors=errors) else: - if utils.is_list_like(labels): + if is_list_like(labels): warnings.warn( "dropping dimensions using list-like labels is deprecated; " "use dict-like arguments.", diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index 23bc2b47e43..043bb7745b3 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -29,6 +29,7 @@ from xarray.core.common import duck_array_ops, full_like from xarray.core.npcompat import IS_NEP18_ACTIVE from xarray.core.pycompat import integer_types +from xarray.core.coordinates import DatasetCoordinates, DataArrayCoordinates from . import ( LooseVersion, @@ -2214,7 +2215,7 @@ def test_drop_labels_by_keyword(self): # Basic functionality. assert len(data.coords["x"]) == 2 - # This API is allowed but deprecated. + # In the future, this will break. with pytest.warns(DeprecationWarning): ds1 = data.drop(["a"], dim="x") ds2 = data.drop(x="a") @@ -2222,6 +2223,13 @@ def test_drop_labels_by_keyword(self): ds4 = data.drop(x=["a", "b"]) ds5 = data.drop(x=["a", "b"], y=range(0, 6, 2)) + # In the future, this will result in different behavior. + arr = DataArray(range(3), dims=["c"]) + with pytest.warns(FutureWarning): + data.drop(arr.coords) + with pytest.warns(FutureWarning): + data.drop(arr.indexes) + assert_array_equal(ds1.coords["x"], ["b"]) assert_array_equal(ds2.coords["x"], ["b"]) assert_array_equal(ds3.coords["x"], ["b"]) From 81c3d7ae8a6b9ac98a2f99c266ccfa23e6b9416f Mon Sep 17 00:00:00 2001 From: Gregory Gundersen Date: Tue, 20 Aug 2019 22:11:16 +0100 Subject: [PATCH 4/6] Moved both warnings to top of function for code clarity. --- xarray/core/dataset.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index e5af6c5204a..71ec81b1729 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -3551,11 +3551,18 @@ def drop( # noqa: F811 if is_dict_like(labels) and not isinstance(labels, dict): warnings.warn( - "dropping coordinates using key values of dict-like labels " - "is deprecated; use drop_vars or a list of coordinates.", + "dropping coordinates using key values of dict-like labels is " + "deprecated; use drop_vars or a list of coordinates.", FutureWarning, stacklevel=2, ) + if dim is not None and is_list_like(labels): + warnings.warn( + "dropping dimensions using list-like labels is deprecated; use " + "dict-like arguments.", + DeprecationWarning, + stacklevel=2, + ) if labels_kwargs or isinstance(labels, dict): labels_kwargs = either_dict_or_kwargs(labels, labels_kwargs, "drop") @@ -3572,13 +3579,6 @@ def drop( # noqa: F811 labels = set(labels) return self._drop_vars(labels, errors=errors) else: - if is_list_like(labels): - warnings.warn( - "dropping dimensions using list-like labels is deprecated; " - "use dict-like arguments.", - DeprecationWarning, - stacklevel=2, - ) return self._drop_labels(labels, dim, errors=errors) def _drop_labels(self, labels=None, dim=None, errors="raise"): From 89f68b5d9767f93f08def8802ee298fe20aff00d Mon Sep 17 00:00:00 2001 From: Gregory Gundersen Date: Tue, 20 Aug 2019 22:13:44 +0100 Subject: [PATCH 5/6] Removed unused imports. --- xarray/tests/test_dataset.py | 1 - 1 file changed, 1 deletion(-) diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index 043bb7745b3..5dfa14671a7 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -29,7 +29,6 @@ from xarray.core.common import duck_array_ops, full_like from xarray.core.npcompat import IS_NEP18_ACTIVE from xarray.core.pycompat import integer_types -from xarray.core.coordinates import DatasetCoordinates, DataArrayCoordinates from . import ( LooseVersion, From d45688e8cefbd776b365f1ba211e0fda420a54ba Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Fri, 23 Aug 2019 16:57:13 -0400 Subject: [PATCH 6/6] Update dataset.py --- xarray/core/dataset.py | 1 - 1 file changed, 1 deletion(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index b7da5651422..9c1a2559c18 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -55,7 +55,6 @@ _contains_datetime_like_objects, ) from .coordinates import ( - DataArrayCoordinates, DatasetCoordinates, LevelCoordinatesSource, assert_coordinate_consistent,