Skip to content

Commit

Permalink
Deprecate inplace (#2524)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
dcherian authored and shoyer committed Nov 3, 2018
1 parent f788084 commit 848d491
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 41 deletions.
3 changes: 3 additions & 0 deletions doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
~~~~~~~~~~~~
Expand Down
15 changes: 10 additions & 5 deletions xarray/core/dataarray.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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``')
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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 = {}
Expand Down
34 changes: 21 additions & 13 deletions xarray/core/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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
Expand All @@ -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 '
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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 = {}
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 '
Expand Down
10 changes: 10 additions & 0 deletions xarray/core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
18 changes: 10 additions & 8 deletions xarray/tests/test_dataarray.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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'):
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand Down
Loading

0 comments on commit 848d491

Please sign in to comment.