From 088443eb31cfe0ab983055aba72f3e8e8526772c Mon Sep 17 00:00:00 2001 From: Justus Magin Date: Tue, 8 Nov 2022 12:02:36 +0100 Subject: [PATCH 01/11] make use of `keep_attrs` in `Variable.pad` --- xarray/core/variable.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/xarray/core/variable.py b/xarray/core/variable.py index 0226f62d45a..986f9ad8c5c 100644 --- a/xarray/core/variable.py +++ b/xarray/core/variable.py @@ -1433,6 +1433,7 @@ def pad( | None = None, end_values: int | tuple[int, int] | Mapping[Any, tuple[int, int]] | None = None, reflect_type: PadReflectOptions = None, + keep_attrs: bool | None = None, **pad_width_kwargs: Any, ): """ @@ -1460,6 +1461,10 @@ def pad( default with an unaltered reflection around the edge value. For the "odd" style, the extended part of the array is created by subtracting the reflected values from two times the edge value. + keep_attrs : bool, optional + If True, the variable's attributes (`attrs`) will be copied from + the original object to the new one. If False (default), the new + object will be returned without attributes. **pad_width_kwargs One of pad_width or pad_width_kwargs must be provided. @@ -1516,7 +1521,11 @@ def pad( **pad_option_kwargs, ) - return type(self)(self.dims, array) + if keep_attrs is None: + keep_attrs = _get_keep_attrs(default=False) + attrs = self._attrs if keep_attrs else None + + return type(self)(self.dims, array, attrs=attrs) def _roll_one_dim(self, dim, count): axis = self.get_axis_num(dim) From 5d5876c00c2ba6b1a45ecf32aeba69bd57b20aa6 Mon Sep 17 00:00:00 2001 From: Justus Magin Date: Tue, 8 Nov 2022 12:06:49 +0100 Subject: [PATCH 02/11] propagate `keep_attrs` --- xarray/core/dataarray.py | 6 ++++++ xarray/core/dataset.py | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/xarray/core/dataarray.py b/xarray/core/dataarray.py index 15d1777b270..de4dfba1210 100644 --- a/xarray/core/dataarray.py +++ b/xarray/core/dataarray.py @@ -5260,6 +5260,7 @@ def pad( | None = None, end_values: int | tuple[int, int] | Mapping[Any, tuple[int, int]] | None = None, reflect_type: PadReflectOptions = None, + keep_attrs: bool | None = None, **pad_width_kwargs: Any, ) -> T_DataArray: """Pad this array along one or more dimensions. @@ -5337,6 +5338,10 @@ def pad( default with an unaltered reflection around the edge value. For the "odd" style, the extended part of the array is created by subtracting the reflected values from two times the edge value. + keep_attrs : bool or None, optional + If True, the attributes (``attrs``) will be copied from the + original object to the new one. If False, the new object + will be returned without attributes. **pad_width_kwargs The keyword arguments form of ``pad_width``. One of ``pad_width`` or ``pad_width_kwargs`` must be provided. @@ -5404,6 +5409,7 @@ def pad( constant_values=constant_values, end_values=end_values, reflect_type=reflect_type, + keep_attrs=keep_attrs, **pad_width_kwargs, ) return self._from_temp_dataset(ds) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index dbf5e46b2ad..4834a0804d3 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -7916,6 +7916,7 @@ def pad( ) = None, end_values: int | tuple[int, int] | Mapping[Any, tuple[int, int]] | None = None, reflect_type: PadReflectOptions = None, + keep_attrs: bool | None = None, **pad_width_kwargs: Any, ) -> T_Dataset: """Pad this dataset along one or more dimensions. @@ -7993,6 +7994,10 @@ def pad( default with an unaltered reflection around the edge value. For the "odd" style, the extended part of the array is created by subtracting the reflected values from two times the edge value. + keep_attrs : bool or None, optional + If True, the attributes (``attrs``) will be copied from the + original object to the new one. If False, the new object + will be returned without attributes. **pad_width_kwargs The keyword arguments form of ``pad_width``. One of ``pad_width`` or ``pad_width_kwargs`` must be provided. @@ -8061,11 +8066,13 @@ def pad( constant_values=constant_values, end_values=end_values, reflect_type=reflect_type, + keep_attrs=keep_attrs, ) else: variables[name] = var.pad( pad_width=var_pad_width, mode=coord_pad_mode, + keep_attrs=keep_attrs, **coord_pad_options, # type: ignore[arg-type] ) # reset default index of dimension coordinates From edc0ff4dcb1ca102d05696b16ef846f5229cc50a Mon Sep 17 00:00:00 2001 From: Justus Magin Date: Tue, 8 Nov 2022 12:08:13 +0100 Subject: [PATCH 03/11] resolve the default `keep_attrs` in `Dataset.pad` --- xarray/core/dataset.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 4834a0804d3..e176e702a0c 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -8044,6 +8044,9 @@ def pad( coord_pad_mode = "constant" coord_pad_options = {} + if keep_attrs is None: + keep_attrs = _get_keep_attrs(default=False) + variables = {} # keep indexes that won't be affected by pad and drop all other indexes From afc8db6dcc0a7669eb7bde213534a969426d3f0c Mon Sep 17 00:00:00 2001 From: Justus Magin Date: Tue, 8 Nov 2022 15:33:56 +0100 Subject: [PATCH 04/11] add tests for `Variable.pad` --- xarray/tests/test_variable.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/xarray/tests/test_variable.py b/xarray/tests/test_variable.py index 983c584f69d..b0d059a3cd0 100644 --- a/xarray/tests/test_variable.py +++ b/xarray/tests/test_variable.py @@ -911,6 +911,28 @@ def test_pad_constant_values(self, xr_arg, np_arg): ) assert_array_equal(actual, expected) + @pytest.mark.parametrize( + ["keep_attrs", "attrs", "expected"], + [ + pytest.param(None, {"a": 1, "b": 2}, {}, id="default"), + pytest.param(False, {"a": 1, "b": 2}, {}, id="False"), + pytest.param(True, {"a": 1, "b": 2}, {"a": 1, "b": 2}, id="True"), + ], + ) + def test_pad_keep_attrs(self, keep_attrs, attrs, expected): + data = np.arange(10, dtype=float) + v = self.cls(["x"], data, attrs) + + keep_attrs_ = "default" if keep_attrs is None else keep_attrs + + with set_options(keep_attrs=keep_attrs_): + actual = v.pad(mode="constant", constant_values=np.nan) + + assert actual.attrs == expected + + actual = v.pad(mode="constant", constant_values=np.nan, keep_attrs=keep_attrs) + assert actual.attrs == expected + @pytest.mark.parametrize("d, w", (("x", 3), ("y", 5))) def test_rolling_window(self, d, w): # Just a working test. See test_nputils for the algorithm validation From 88240a3e79c8b11b9bcdf0d1e3990d8b437e373e Mon Sep 17 00:00:00 2001 From: Justus Magin Date: Tue, 8 Nov 2022 15:47:01 +0100 Subject: [PATCH 05/11] add tests for `Dataset.pad` and keep the `Dataset` attrs --- xarray/core/dataset.py | 3 ++- xarray/tests/test_dataset.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index e176e702a0c..4cc25b83ebc 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -8086,7 +8086,8 @@ def pad( indexes[name] = index variables[name] = index_vars[name] - return self._replace_with_new_dims(variables, indexes=indexes) + attrs = self._attrs if keep_attrs else None + return self._replace_with_new_dims(variables, indexes=indexes, attrs=attrs) def idxmin( self: T_Dataset, diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index 23ea705db71..bee90244765 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -6102,6 +6102,37 @@ def test_pad(self) -> None: np.testing.assert_equal(padded["var1"].isel(dim2=[0, -1]).data, 42) np.testing.assert_equal(padded["dim2"][[0, -1]].data, np.nan) + @pytest.mark.parametrize( + ["keep_attrs", "attrs", "expected"], + [ + pytest.param(None, {"a": 1, "b": 2}, {}, id="default"), + pytest.param(False, {"a": 1, "b": 2}, {}, id="False"), + pytest.param(True, {"a": 1, "b": 2}, {"a": 1, "b": 2}, id="True"), + ], + ) + def test_pad_keep_attrs(self, keep_attrs, attrs, expected) -> None: + ds = xr.Dataset( + {"a": ("x", [1, 2], attrs)}, + coords={"c": ("x", [-1, 1], attrs)}, + attrs=attrs, + ) + expected = xr.Dataset( + {"a": ("x", [0, 1, 2, 0], expected)}, + coords={"c": ("x", [np.nan, -1, 1, np.nan], expected)}, + attrs=expected, + ) + + keep_attrs_ = "default" if keep_attrs is None else keep_attrs + + with set_options(keep_attrs=keep_attrs_): + actual = ds.pad({"x": (1, 1)}, mode="constant", constant_values=0) + xr.testing.assert_identical(actual, expected) + + actual = ds.pad( + {"x": (1, 1)}, mode="constant", constant_values=0, keep_attrs=keep_attrs + ) + xr.testing.assert_identical(actual, expected) + def test_astype_attrs(self) -> None: data = create_test_data(seed=123) data.attrs["foo"] = "bar" From 56c5011ff33d8e083a68607e5e60273691437c17 Mon Sep 17 00:00:00 2001 From: Justus Magin Date: Tue, 8 Nov 2022 15:48:16 +0100 Subject: [PATCH 06/11] actually pad the variable --- xarray/tests/test_variable.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/xarray/tests/test_variable.py b/xarray/tests/test_variable.py index b0d059a3cd0..a9e06db0d03 100644 --- a/xarray/tests/test_variable.py +++ b/xarray/tests/test_variable.py @@ -926,11 +926,16 @@ def test_pad_keep_attrs(self, keep_attrs, attrs, expected): keep_attrs_ = "default" if keep_attrs is None else keep_attrs with set_options(keep_attrs=keep_attrs_): - actual = v.pad(mode="constant", constant_values=np.nan) + actual = v.pad({"x": (1, 1)}, mode="constant", constant_values=np.nan) assert actual.attrs == expected - actual = v.pad(mode="constant", constant_values=np.nan, keep_attrs=keep_attrs) + actual = v.pad( + {"x": (1, 1)}, + mode="constant", + constant_values=np.nan, + keep_attrs=keep_attrs, + ) assert actual.attrs == expected @pytest.mark.parametrize("d, w", (("x", 3), ("y", 5))) From db97568befde11246009f957d37fc2c7f918c4d9 Mon Sep 17 00:00:00 2001 From: Justus Magin Date: Tue, 8 Nov 2022 15:50:20 +0100 Subject: [PATCH 07/11] also check that untouched variables are not modified --- xarray/tests/test_dataset.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index bee90244765..b5237a630c8 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -6112,13 +6112,16 @@ def test_pad(self) -> None: ) def test_pad_keep_attrs(self, keep_attrs, attrs, expected) -> None: ds = xr.Dataset( - {"a": ("x", [1, 2], attrs)}, - coords={"c": ("x", [-1, 1], attrs)}, + {"a": ("x", [1, 2], attrs), "b": ("y", [1, 2], attrs)}, + coords={"c": ("x", [-1, 1], attrs), "d": ("y", [-1, 1], attrs)}, attrs=attrs, ) expected = xr.Dataset( - {"a": ("x", [0, 1, 2, 0], expected)}, - coords={"c": ("x", [np.nan, -1, 1, np.nan], expected)}, + {"a": ("x", [0, 1, 2, 0], expected), "b": ("y", [1, 2], attrs)}, + coords={ + "c": ("x", [np.nan, -1, 1, np.nan], expected), + "d": ("y", [-1, 1], attrs), + }, attrs=expected, ) From 527ff2799590d673d6d3ac78969a5f07d799020d Mon Sep 17 00:00:00 2001 From: Justus Magin Date: Tue, 8 Nov 2022 15:53:26 +0100 Subject: [PATCH 08/11] add tests for `DataArray.pad` --- xarray/tests/test_dataarray.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/xarray/tests/test_dataarray.py b/xarray/tests/test_dataarray.py index ac6049872b8..a9e3b52f348 100644 --- a/xarray/tests/test_dataarray.py +++ b/xarray/tests/test_dataarray.py @@ -4166,6 +4166,36 @@ def test_pad_reflect(self, mode, reflect_type) -> None: assert actual.shape == (7, 4, 9) assert_identical(actual, expected) + @pytest.mark.parametrize( + ["keep_attrs", "attrs", "expected"], + [ + pytest.param(None, {"a": 1, "b": 2}, {}, id="default"), + pytest.param(False, {"a": 1, "b": 2}, {}, id="False"), + pytest.param(True, {"a": 1, "b": 2}, {"a": 1, "b": 2}, id="True"), + ], + ) + def test_pad_keep_attrs(self, keep_attrs, attrs, expected) -> None: + arr = xr.DataArray( + [1, 2], dims="x", coords={"c": ("x", [-1, 1], attrs)}, attrs=attrs + ) + expected = xr.DataArray( + [0, 1, 2, 0], + dims="x", + coords={"c": ("x", [np.nan, -1, 1, np.nan], expected)}, + attrs=expected, + ) + + keep_attrs_ = "default" if keep_attrs is None else keep_attrs + + with set_options(keep_attrs=keep_attrs_): + actual = arr.pad({"x": (1, 1)}, mode="constant", constant_values=0) + xr.testing.assert_identical(actual, expected) + + actual = arr.pad( + {"x": (1, 1)}, mode="constant", constant_values=0, keep_attrs=keep_attrs + ) + xr.testing.assert_identical(actual, expected) + @pytest.mark.parametrize("parser", ["pandas", "python"]) @pytest.mark.parametrize( "engine", ["python", None, pytest.param("numexpr", marks=[requires_numexpr])] From 69b1bd85a96fdd7f8a0c13d7f37a76a307cd0622 Mon Sep 17 00:00:00 2001 From: Justus Magin Date: Tue, 8 Nov 2022 15:56:54 +0100 Subject: [PATCH 09/11] update whats-new.rst --- doc/whats-new.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 690b1336e31..6a2992f3f1c 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -34,7 +34,9 @@ Deprecations Bug fixes ~~~~~~~~~ - +- add a ``keep_attrs`` parameter to :py:meth:`Dataset.pad`, :py:meth:`DataArray.pad`, + and :py:meth:`Variable.pad` (:pull:`7267`). + By `Justus Magin `_. Documentation ~~~~~~~~~~~~~ From 6428cadbb43c49c185bb304923f5cf3718cec3c2 Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Thu, 1 Dec 2022 09:23:49 -0700 Subject: [PATCH 10/11] Set True by default --- xarray/core/dataset.py | 2 +- xarray/core/variable.py | 2 +- xarray/tests/test_dataarray.py | 2 +- xarray/tests/test_dataset.py | 2 +- xarray/tests/test_variable.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 4cc25b83ebc..0e941293d99 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -8045,7 +8045,7 @@ def pad( coord_pad_options = {} if keep_attrs is None: - keep_attrs = _get_keep_attrs(default=False) + keep_attrs = _get_keep_attrs(default=True) variables = {} diff --git a/xarray/core/variable.py b/xarray/core/variable.py index 986f9ad8c5c..72f6ee6d675 100644 --- a/xarray/core/variable.py +++ b/xarray/core/variable.py @@ -1522,7 +1522,7 @@ def pad( ) if keep_attrs is None: - keep_attrs = _get_keep_attrs(default=False) + keep_attrs = _get_keep_attrs(default=True) attrs = self._attrs if keep_attrs else None return type(self)(self.dims, array, attrs=attrs) diff --git a/xarray/tests/test_dataarray.py b/xarray/tests/test_dataarray.py index a9e3b52f348..443a93c3de4 100644 --- a/xarray/tests/test_dataarray.py +++ b/xarray/tests/test_dataarray.py @@ -4169,7 +4169,7 @@ def test_pad_reflect(self, mode, reflect_type) -> None: @pytest.mark.parametrize( ["keep_attrs", "attrs", "expected"], [ - pytest.param(None, {"a": 1, "b": 2}, {}, id="default"), + pytest.param(None, {"a": 1, "b": 2}, {"a": 1, "b": 2}, id="default"), pytest.param(False, {"a": 1, "b": 2}, {}, id="False"), pytest.param(True, {"a": 1, "b": 2}, {"a": 1, "b": 2}, id="True"), ], diff --git a/xarray/tests/test_dataset.py b/xarray/tests/test_dataset.py index b5237a630c8..b5b261807be 100644 --- a/xarray/tests/test_dataset.py +++ b/xarray/tests/test_dataset.py @@ -6105,7 +6105,7 @@ def test_pad(self) -> None: @pytest.mark.parametrize( ["keep_attrs", "attrs", "expected"], [ - pytest.param(None, {"a": 1, "b": 2}, {}, id="default"), + pytest.param(None, {"a": 1, "b": 2}, {"a": 1, "b": 2}, id="default"), pytest.param(False, {"a": 1, "b": 2}, {}, id="False"), pytest.param(True, {"a": 1, "b": 2}, {"a": 1, "b": 2}, id="True"), ], diff --git a/xarray/tests/test_variable.py b/xarray/tests/test_variable.py index a9e06db0d03..e099353539d 100644 --- a/xarray/tests/test_variable.py +++ b/xarray/tests/test_variable.py @@ -914,7 +914,7 @@ def test_pad_constant_values(self, xr_arg, np_arg): @pytest.mark.parametrize( ["keep_attrs", "attrs", "expected"], [ - pytest.param(None, {"a": 1, "b": 2}, {}, id="default"), + pytest.param(None, {"a": 1, "b": 2}, {"a": 1, "b": 2}, id="default"), pytest.param(False, {"a": 1, "b": 2}, {}, id="False"), pytest.param(True, {"a": 1, "b": 2}, {"a": 1, "b": 2}, id="True"), ], From 192a17b96fc107b112e51afafdf6852e5aac6f4f Mon Sep 17 00:00:00 2001 From: Justus Magin Date: Mon, 12 Dec 2022 16:38:44 +0100 Subject: [PATCH 11/11] move the whats-new.rst entry to the dev section --- doc/whats-new.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index e29e6247126..e2f0cfcc5f6 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -36,6 +36,9 @@ Bug fixes ~~~~~~~~~ - Allow numpy-only objects in :py:func:`where` when ``keep_attrs=True`` (:issue:`7362`, :pull:`7364`). By `Sam Levang `_. +- add a ``keep_attrs`` parameter to :py:meth:`Dataset.pad`, :py:meth:`DataArray.pad`, + and :py:meth:`Variable.pad` (:pull:`7267`). + By `Justus Magin `_. Documentation ~~~~~~~~~~~~~ @@ -95,9 +98,6 @@ Deprecations Bug fixes ~~~~~~~~~ -- add a ``keep_attrs`` parameter to :py:meth:`Dataset.pad`, :py:meth:`DataArray.pad`, - and :py:meth:`Variable.pad` (:pull:`7267`). - By `Justus Magin `_. - Fix handling of coordinate attributes in :py:func:`where`. (:issue:`7220`, :pull:`7229`) By `Sam Levang `_. - Import ``nc_time_axis`` when needed (:issue:`7275`, :pull:`7276`).