Skip to content

remove all inplace parameters from the public API #4615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions xarray/core/dataarray.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
from .indexing import is_fancy_indexer
from .merge import PANDAS_TYPES, MergeError, _extract_indexes_from_coords
from .options import OPTIONS, _get_keep_attrs
from .utils import Default, ReprObject, _check_inplace, _default, either_dict_or_kwargs
from .utils import Default, ReprObject, _default, either_dict_or_kwargs
from .variable import (
IndexVariable,
Variable,
Expand Down Expand Up @@ -778,7 +778,6 @@ def reset_coords(
self,
names: Union[Iterable[Hashable], Hashable, None] = None,
drop: bool = False,
inplace: bool = None,
) -> Union[None, "DataArray", Dataset]:
"""Given names of coordinates, reset them to become variables.

Expand All @@ -795,7 +794,6 @@ def reset_coords(
-------
Dataset, or DataArray if ``drop == True``
"""
_check_inplace(inplace)
if names is None:
names = set(self.coords) - set(self.dims)
dataset = self.coords.to_dataset().reset_coords(names, drop)
Expand Down Expand Up @@ -1804,7 +1802,6 @@ def set_index(
self,
indexes: Mapping[Hashable, Union[Hashable, Sequence[Hashable]]] = None,
append: bool = False,
inplace: bool = None,
**indexes_kwargs: Union[Hashable, Sequence[Hashable]],
) -> Optional["DataArray"]:
"""Set DataArray (multi-)indexes using one or more existing
Expand Down Expand Up @@ -1855,16 +1852,13 @@ def set_index(
--------
DataArray.reset_index
"""
ds = self._to_temp_dataset().set_index(
indexes, append=append, inplace=inplace, **indexes_kwargs
)
ds = self._to_temp_dataset().set_index(indexes, append=append, **indexes_kwargs)
return self._from_temp_dataset(ds)

def reset_index(
self,
dims_or_levels: Union[Hashable, Sequence[Hashable]],
drop: bool = False,
inplace: bool = None,
) -> Optional["DataArray"]:
"""Reset the specified index(es) or multi-index level(s).

Expand All @@ -1887,7 +1881,6 @@ def reset_index(
--------
DataArray.set_index
"""
_check_inplace(inplace)
coords, _ = split_indexes(
dims_or_levels, self._coords, set(), self._level_coords, drop=drop
)
Expand All @@ -1896,7 +1889,6 @@ def reset_index(
def reorder_levels(
self,
dim_order: Mapping[Hashable, Sequence[int]] = None,
inplace: bool = None,
**dim_order_kwargs: Sequence[int],
) -> "DataArray":
"""Rearrange index levels using input order.
Expand All @@ -1917,7 +1909,6 @@ def reorder_levels(
Another dataarray, with this dataarray's data but replaced
coordinates.
"""
_check_inplace(inplace)
dim_order = either_dict_or_kwargs(dim_order, dim_order_kwargs, "reorder_levels")
replace_coords = {}
for dim, order in dim_order.items():
Expand Down
24 changes: 3 additions & 21 deletions xarray/core/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
Default,
Frozen,
SortedKeysDict,
_check_inplace,
_default,
decode_numpy_dict_values,
drop_dims_from_indexers,
Expand Down Expand Up @@ -1504,9 +1503,7 @@ def data_vars(self) -> DataVariables:
"""Dictionary of DataArray objects corresponding to data variables"""
return DataVariables(self)

def set_coords(
self, names: "Union[Hashable, Iterable[Hashable]]", inplace: bool = None
) -> "Dataset":
def set_coords(self, names: "Union[Hashable, Iterable[Hashable]]") -> "Dataset":
"""Given names of one or more variables, set them as coordinates

Parameters
Expand All @@ -1526,7 +1523,6 @@ def set_coords(
# DataFrame.set_index?
# nb. check in self._variables, not self.data_vars to insure that the
# operation is idempotent
_check_inplace(inplace)
if isinstance(names, str) or not isinstance(names, Iterable):
names = [names]
else:
Expand All @@ -1540,7 +1536,6 @@ def reset_coords(
self,
names: "Union[Hashable, Iterable[Hashable], None]" = None,
drop: bool = False,
inplace: bool = None,
) -> "Dataset":
"""Given names of coordinates, reset them to become variables

Expand All @@ -1557,7 +1552,6 @@ def reset_coords(
-------
Dataset
"""
_check_inplace(inplace)
if names is None:
names = self._coord_names - set(self.dims)
else:
Expand Down Expand Up @@ -3124,9 +3118,7 @@ def rename_vars(
)
return self._replace(variables, coord_names, dims=dims, indexes=indexes)

def swap_dims(
self, dims_dict: Mapping[Hashable, Hashable], inplace: bool = None
) -> "Dataset":
def swap_dims(self, dims_dict: Mapping[Hashable, Hashable]) -> "Dataset":
"""Returns a new object with swapped dimensions.

Parameters
Expand Down Expand Up @@ -3185,7 +3177,6 @@ def swap_dims(
"""
# TODO: deprecate this method in favor of a (less confusing)
# rename_dims() method that only renames dimensions.
_check_inplace(inplace)
for k, v in dims_dict.items():
if k not in self.dims:
raise ValueError(
Expand Down Expand Up @@ -3360,7 +3351,6 @@ def set_index(
self,
indexes: Mapping[Hashable, Union[Hashable, Sequence[Hashable]]] = None,
append: bool = False,
inplace: bool = None,
**indexes_kwargs: Union[Hashable, Sequence[Hashable]],
) -> "Dataset":
"""Set Dataset (multi-)indexes using one or more existing coordinates
Expand Down Expand Up @@ -3415,7 +3405,6 @@ def set_index(
Dataset.reset_index
Dataset.swap_dims
"""
_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
Expand All @@ -3426,7 +3415,6 @@ def reset_index(
self,
dims_or_levels: Union[Hashable, Sequence[Hashable]],
drop: bool = False,
inplace: bool = None,
) -> "Dataset":
"""Reset the specified index(es) or multi-index level(s).

Expand All @@ -3448,7 +3436,6 @@ def reset_index(
--------
Dataset.set_index
"""
_check_inplace(inplace)
variables, coord_names = split_indexes(
dims_or_levels,
self._variables,
Expand All @@ -3461,7 +3448,6 @@ def reset_index(
def reorder_levels(
self,
dim_order: Mapping[Hashable, Sequence[int]] = None,
inplace: bool = None,
**dim_order_kwargs: Sequence[int],
) -> "Dataset":
"""Rearrange index levels using input order.
Expand All @@ -3482,7 +3468,6 @@ def reorder_levels(
Another dataset, with this dataset's data but replaced
coordinates.
"""
_check_inplace(inplace)
dim_order = either_dict_or_kwargs(dim_order, dim_order_kwargs, "reorder_levels")
variables = self._variables.copy()
indexes = dict(self.indexes)
Expand Down Expand Up @@ -3785,7 +3770,7 @@ def unstack(
result = result._unstack_once(dim, fill_value, sparse)
return result

def update(self, other: "CoercibleMapping", inplace: bool = None) -> "Dataset":
def update(self, other: "CoercibleMapping") -> "Dataset":
"""Update this dataset's variables with those from another dataset.

Parameters
Expand All @@ -3811,14 +3796,12 @@ def update(self, other: "CoercibleMapping", inplace: bool = None) -> "Dataset":
If any dimensions would have inconsistent sizes in the updated
dataset.
"""
_check_inplace(inplace)
merge_result = dataset_update_method(self, other)
return self._replace(inplace=True, **merge_result._asdict())

def merge(
self,
other: Union["CoercibleMapping", "DataArray"],
inplace: bool = None,
overwrite_vars: Union[Hashable, Iterable[Hashable]] = frozenset(),
compat: str = "no_conflicts",
join: str = "outer",
Expand Down Expand Up @@ -3874,7 +3857,6 @@ def merge(
MergeError
If any variables conflict (see ``compat``).
"""
_check_inplace(inplace)
other = other.to_dataset() if isinstance(other, xr.DataArray) else other
merge_result = dataset_merge_method(
self,
Expand Down
8 changes: 0 additions & 8 deletions xarray/core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,6 @@
T = TypeVar("T")


def _check_inplace(inplace: Optional[bool]) -> None:
if inplace is not None:
raise TypeError(
"The `inplace` argument has been removed from xarray. "
"You can achieve an identical effect with python's standard assignment."
)


def alias_message(old_name: str, new_name: str) -> str:
return f"{old_name} has been deprecated. Use {new_name} instead."

Expand Down
6 changes: 0 additions & 6 deletions xarray/tests/test_dataarray.py
Original file line number Diff line number Diff line change
Expand Up @@ -1405,8 +1405,6 @@ def test_reset_coords(self):
)
assert_identical(actual, expected)

with pytest.raises(TypeError):
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 @@ -1871,10 +1869,6 @@ def test_reorder_levels(self):
obj = self.mda.reorder_levels(x=["level_2", "level_1"])
assert_identical(obj, expected)

with pytest.raises(TypeError):
array = self.mda.copy()
array.reorder_levels(x=["level_2", "level_1"], inplace=True)

array = DataArray([1, 2], dims="x")
with pytest.raises(KeyError):
array.reorder_levels(x=["level_1", "level_2"])
Expand Down
13 changes: 0 additions & 13 deletions xarray/tests/test_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -2889,10 +2889,6 @@ def test_set_index(self):
obj = ds.set_index(x=mindex.names)
assert_identical(obj, expected)

with pytest.raises(TypeError):
ds.set_index(x=mindex.names, inplace=True)
assert_identical(ds, expected)

# ensure set_index with no existing index and a single data var given
# doesn't return multi-index
ds = Dataset(data_vars={"x_var": ("x", [0, 1, 2])})
Expand All @@ -2914,9 +2910,6 @@ def test_reset_index(self):
obj = ds.reset_index("x")
assert_identical(obj, expected)

with pytest.raises(TypeError):
ds.reset_index("x", inplace=True)

def test_reset_index_keep_attrs(self):
coord_1 = DataArray([1, 2], dims=["coord_1"], attrs={"attrs": True})
ds = Dataset({}, {"coord_1": coord_1})
Expand All @@ -2933,9 +2926,6 @@ def test_reorder_levels(self):
reindexed = ds.reorder_levels(x=["level_2", "level_1"])
assert_identical(reindexed, expected)

with pytest.raises(TypeError):
ds.reorder_levels(x=["level_2", "level_1"], inplace=True)

ds = Dataset({}, coords={"x": [1, 2]})
with raises_regex(ValueError, "has no MultiIndex"):
ds.reorder_levels(x=["level_1", "level_2"])
Expand Down Expand Up @@ -3133,9 +3123,6 @@ def test_update(self):
assert actual_result is actual
assert_identical(expected, actual)

with pytest.raises(TypeError):
actual = data.update(data, inplace=False)

other = Dataset(attrs={"new": "attr"})
actual = data.copy()
actual.update(other)
Expand Down