Deprecate inplace (#2524)

* Start deprecating inplace.

* remove warnings from tests.

* this commit silences nearly all warnings.

* Add whats-new.

* Add a default kwarg to _check_inplace and use for Dataset.update.

* Major fix!

* Add stacklevel

* Tests: Less aggressive warning filter + fix unnecessary inplace.

* revert changes to _calculate_binary_op

Author: dcherian

doc/whats-new.rst

@@ -77,6 +77,9 @@ Documentation
   without dimension argument will change in the next release.
   Now we warn a FutureWarning.
   By `Keisuke Fujii <https://github.com/fujiisoup>`_.
+- The ``inplace`` kwarg of a number of `DataArray` and `Dataset` methods is being
+  deprecated and will be removed in the next release.
+  By `Deepak Cherian <https://github.com/dcherian>`_.
 
 Enhancements
 ~~~~~~~~~~~~

xarray/core/dataarray.py

@@ -19,7 +19,8 @@
 from .options import OPTIONS, _get_keep_attrs
 from .pycompat import OrderedDict, basestring, iteritems, range, zip
 from .utils import (
-    decode_numpy_dict_values, either_dict_or_kwargs, ensure_us_time_resolution)
+    _check_inplace, decode_numpy_dict_values, either_dict_or_kwargs,
+    ensure_us_time_resolution)
 from .variable import (
     IndexVariable, Variable, as_compatible_data, as_variable,
     assert_unique_multiindex_level_names)
@@ -542,7 +543,7 @@ def coords(self):
         """
         return DataArrayCoordinates(self)
 
-    def reset_coords(self, names=None, drop=False, inplace=False):
+    def reset_coords(self, names=None, drop=False, inplace=None):
         """Given names of coordinates, reset them to become variables.
 
         Parameters
@@ -561,6 +562,7 @@ def reset_coords(self, names=None, drop=False, inplace=False):
         -------
         Dataset, or DataArray if ``drop == True``
         """
+        inplace = _check_inplace(inplace)
         if inplace and not drop:
             raise ValueError('cannot reset coordinates in-place on a '
                              'DataArray without ``drop == True``')
@@ -1151,7 +1153,7 @@ def expand_dims(self, dim, axis=None):
         ds = self._to_temp_dataset().expand_dims(dim, axis)
         return self._from_temp_dataset(ds)
 
-    def set_index(self, indexes=None, append=False, inplace=False,
+    def set_index(self, indexes=None, append=False, inplace=None,
                   **indexes_kwargs):
         """Set DataArray (multi-)indexes using one or more existing
         coordinates.
@@ -1181,14 +1183,15 @@ def set_index(self, indexes=None, append=False, inplace=False,
         --------
         DataArray.reset_index
         """
+        inplace = _check_inplace(inplace)
         indexes = either_dict_or_kwargs(indexes, indexes_kwargs, 'set_index')
         coords, _ = merge_indexes(indexes, self._coords, set(), append=append)
         if inplace:
             self._coords = coords
         else:
             return self._replace(coords=coords)
 
-    def reset_index(self, dims_or_levels, drop=False, inplace=False):
+    def reset_index(self, dims_or_levels, drop=False, inplace=None):
         """Reset the specified index(es) or multi-index level(s).
 
         Parameters
@@ -1213,14 +1216,15 @@ def reset_index(self, dims_or_levels, drop=False, inplace=False):
         --------
         DataArray.set_index
         """
+        inplace = _check_inplace(inplace)
         coords, _ = split_indexes(dims_or_levels, self._coords, set(),
                                   self._level_coords, drop=drop)
         if inplace:
             self._coords = coords
         else:
             return self._replace(coords=coords)
 
-    def reorder_levels(self, dim_order=None, inplace=False,
+    def reorder_levels(self, dim_order=None, inplace=None,
                        **dim_order_kwargs):
         """Rearrange index levels using input order.
 
@@ -1243,6 +1247,7 @@ def reorder_levels(self, dim_order=None, inplace=False,
             Another dataarray, with this dataarray's data but replaced
             coordinates.
         """
+        inplace = _check_inplace(inplace)
         dim_order = either_dict_or_kwargs(dim_order, dim_order_kwargs,
                                           'reorder_levels')
         replace_coords = {}

xarray/core/dataset.py

@@ -32,9 +32,9 @@
 from .pycompat import (
     OrderedDict, basestring, dask_array_type, integer_types, iteritems, range)
 from .utils import (
-    Frozen, SortedKeysDict, datetime_to_numeric, decode_numpy_dict_values,
-    either_dict_or_kwargs, ensure_us_time_resolution, hashable,
-    maybe_wrap_array)
+    _check_inplace, Frozen, SortedKeysDict, datetime_to_numeric,
+    decode_numpy_dict_values, either_dict_or_kwargs, ensure_us_time_resolution,
+    hashable, maybe_wrap_array)
 from .variable import IndexVariable, Variable, as_variable, broadcast_variables
 
 # list of attributes of pd.DatetimeIndex that are ndarrays of time info
@@ -1072,7 +1072,7 @@ def data_vars(self):
         """
         return DataVariables(self)
 
-    def set_coords(self, names, inplace=False):
+    def set_coords(self, names, inplace=None):
         """Given names of one or more variables, set them as coordinates
 
         Parameters
@@ -1095,14 +1095,15 @@ def set_coords(self, names, inplace=False):
         # DataFrame.set_index?
         # nb. check in self._variables, not self.data_vars to insure that the
         # operation is idempotent
+        inplace = _check_inplace(inplace)
         if isinstance(names, basestring):
             names = [names]
         self._assert_all_in_dataset(names)
         obj = self if inplace else self.copy()
         obj._coord_names.update(names)
         return obj
 
-    def reset_coords(self, names=None, drop=False, inplace=False):
+    def reset_coords(self, names=None, drop=False, inplace=None):
         """Given names of coordinates, reset them to become variables
 
         Parameters
@@ -1121,6 +1122,7 @@ def reset_coords(self, names=None, drop=False, inplace=False):
         -------
         Dataset
         """
+        inplace = _check_inplace(inplace)
         if names is None:
             names = self._coord_names - set(self.dims)
         else:
@@ -2045,7 +2047,7 @@ def interp_like(self, other, method='linear', assume_sorted=False,
             ds = self.reindex(object_coords)
         return ds.interp(numeric_coords, method, assume_sorted, kwargs)
 
-    def rename(self, name_dict=None, inplace=False, **names):
+    def rename(self, name_dict=None, inplace=None, **names):
         """Returns a new object with renamed variables and dimensions.
 
         Parameters
@@ -2070,6 +2072,7 @@ def rename(self, name_dict=None, inplace=False, **names):
         Dataset.swap_dims
         DataArray.rename
         """
+        inplace = _check_inplace(inplace)
         name_dict = either_dict_or_kwargs(name_dict, names, 'rename')
         for k, v in name_dict.items():
             if k not in self and k not in self.dims:
@@ -2095,7 +2098,7 @@ def rename(self, name_dict=None, inplace=False, **names):
         return self._replace_vars_and_dims(variables, coord_names, dims=dims,
                                            inplace=inplace)
 
-    def swap_dims(self, dims_dict, inplace=False):
+    def swap_dims(self, dims_dict, inplace=None):
         """Returns a new object with swapped dimensions.
 
         Parameters
@@ -2119,6 +2122,7 @@ def swap_dims(self, dims_dict, inplace=False):
         Dataset.rename
         DataArray.swap_dims
         """
+        inplace = _check_inplace(inplace)
         for k, v in dims_dict.items():
             if k not in self.dims:
                 raise ValueError('cannot swap from dimension %r because it is '
@@ -2231,7 +2235,7 @@ def expand_dims(self, dim, axis=None):
 
         return self._replace_vars_and_dims(variables, self._coord_names)
 
-    def set_index(self, indexes=None, append=False, inplace=False,
+    def set_index(self, indexes=None, append=False, inplace=None,
                   **indexes_kwargs):
         """Set Dataset (multi-)indexes using one or more existing coordinates or
         variables.
@@ -2262,14 +2266,15 @@ def set_index(self, indexes=None, append=False, inplace=False,
         Dataset.reset_index
         Dataset.swap_dims
         """
+        inplace = _check_inplace(inplace)
         indexes = either_dict_or_kwargs(indexes, indexes_kwargs, 'set_index')
         variables, coord_names = merge_indexes(indexes, self._variables,
                                                self._coord_names,
                                                append=append)
         return self._replace_vars_and_dims(variables, coord_names=coord_names,
                                            inplace=inplace)
 
-    def reset_index(self, dims_or_levels, drop=False, inplace=False):
+    def reset_index(self, dims_or_levels, drop=False, inplace=None):
         """Reset the specified index(es) or multi-index level(s).
 
         Parameters
@@ -2293,13 +2298,14 @@ def reset_index(self, dims_or_levels, drop=False, inplace=False):
         --------
         Dataset.set_index
         """
+        inplace = _check_inplace(inplace)
         variables, coord_names = split_indexes(dims_or_levels, self._variables,
                                                self._coord_names,
                                                self._level_coords, drop=drop)
         return self._replace_vars_and_dims(variables, coord_names=coord_names,
                                            inplace=inplace)
 
-    def reorder_levels(self, dim_order=None, inplace=False,
+    def reorder_levels(self, dim_order=None, inplace=None,
                        **dim_order_kwargs):
         """Rearrange index levels using input order.
 
@@ -2322,6 +2328,7 @@ def reorder_levels(self, dim_order=None, inplace=False,
             Another dataset, with this dataset's data but replaced
             coordinates.
         """
+        inplace = _check_inplace(inplace)
         dim_order = either_dict_or_kwargs(dim_order, dim_order_kwargs,
                                           'reorder_levels')
         replace_variables = {}
@@ -2472,7 +2479,7 @@ def unstack(self, dim=None):
             result = result._unstack_once(dim)
         return result
 
-    def update(self, other, inplace=True):
+    def update(self, other, inplace=None):
         """Update this dataset's variables with those from another dataset.
 
         Parameters
@@ -2494,12 +2501,13 @@ def update(self, other, inplace=True):
             If any dimensions would have inconsistent sizes in the updated
             dataset.
         """
+        inplace = _check_inplace(inplace, default=True)
         variables, coord_names, dims = dataset_update_method(self, other)
 
         return self._replace_vars_and_dims(variables, coord_names, dims,
                                            inplace=inplace)
 
-    def merge(self, other, inplace=False, overwrite_vars=frozenset(),
+    def merge(self, other, inplace=None, overwrite_vars=frozenset(),
               compat='no_conflicts', join='outer'):
         """Merge the arrays of two datasets into a single dataset.
 
@@ -2550,6 +2558,7 @@ def merge(self, other, inplace=False, overwrite_vars=frozenset(),
         MergeError
             If any variables conflict (see ``compat``).
         """
+        inplace = _check_inplace(inplace)
         variables, coord_names, dims = dataset_merge_method(
             self, other, overwrite_vars=overwrite_vars, compat=compat,
             join=join)
@@ -3317,7 +3326,6 @@ def func(self, other):
 
     def _calculate_binary_op(self, f, other, join='inner',
                              inplace=False):
-
         def apply_over_both(lhs_data_vars, rhs_data_vars, lhs_vars, rhs_vars):
             if inplace and set(lhs_data_vars) != set(rhs_data_vars):
                 raise ValueError('datasets must have the same data variables '

xarray/core/utils.py

@@ -17,6 +17,16 @@
     OrderedDict, basestring, bytes_type, dask_array_type, iteritems)
 
 
+def _check_inplace(inplace, default=False):
+    if inplace is None:
+        inplace = default
+    else:
+        warnings.warn('The inplace argument has been deprecated and will be '
+                      'removed in xarray 0.12.0.', FutureWarning, stacklevel=3)
+
+    return inplace
+
+
 def alias_message(old_name, new_name):
     return '%s has been deprecated. Use %s instead.' % (old_name, new_name)
 

xarray/tests/test_dataarray.py

@@ -1155,7 +1155,7 @@ def test_reset_coords(self):
         assert_identical(actual, expected)
 
         actual = data.copy()
-        actual.reset_coords(drop=True, inplace=True)
+        actual = actual.reset_coords(drop=True)
         assert_identical(actual, expected)
 
         actual = data.reset_coords('bar', drop=True)
@@ -1164,8 +1164,9 @@ def test_reset_coords(self):
                              dims=['x', 'y'], name='foo')
         assert_identical(actual, expected)
 
-        with raises_regex(ValueError, 'cannot reset coord'):
-            data.reset_coords(inplace=True)
+        with pytest.warns(FutureWarning, message='The inplace argument'):
+            with raises_regex(ValueError, 'cannot reset coord'):
+                data = data.reset_coords(inplace=True)
         with raises_regex(ValueError, 'cannot be found'):
             data.reset_coords('foo', drop=True)
         with raises_regex(ValueError, 'cannot be found'):
@@ -1398,7 +1399,7 @@ def test_set_index(self):
         expected = array.set_index(x=['level_1', 'level_2', 'level_3'])
         assert_identical(obj, expected)
 
-        array.set_index(x=['level_1', 'level_2', 'level_3'], inplace=True)
+        array = array.set_index(x=['level_1', 'level_2', 'level_3'])
         assert_identical(array, expected)
 
         array2d = DataArray(np.random.rand(2, 2),
@@ -1431,7 +1432,7 @@ def test_reset_index(self):
         assert_identical(obj, expected)
 
         array = self.mda.copy()
-        array.reset_index(['x'], drop=True, inplace=True)
+        array = array.reset_index(['x'], drop=True)
         assert_identical(array, expected)
 
         # single index
@@ -1447,9 +1448,10 @@ def test_reorder_levels(self):
         obj = self.mda.reorder_levels(x=['level_2', 'level_1'])
         assert_identical(obj, expected)
 
-        array = self.mda.copy()
-        array.reorder_levels(x=['level_2', 'level_1'], inplace=True)
-        assert_identical(array, expected)
+        with pytest.warns(FutureWarning, message='The inplace argument'):
+            array = self.mda.copy()
+            array.reorder_levels(x=['level_2', 'level_1'], inplace=True)
+            assert_identical(array, expected)
 
         array = DataArray([1, 2], dims='x')
         with pytest.raises(KeyError):