From 2aeb1c9f943aa9df55a898686361f35b0ac70c17 Mon Sep 17 00:00:00 2001 From: tomvothecoder Date: Thu, 14 Sep 2023 11:42:05 -0700 Subject: [PATCH 1/5] Add deperecation warnings for `add_bounds` boolean args --- xcdat/dataset.py | 59 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 14 deletions(-) diff --git a/xcdat/dataset.py b/xcdat/dataset.py index 7dc64a85..dd321bb1 100644 --- a/xcdat/dataset.py +++ b/xcdat/dataset.py @@ -46,7 +46,7 @@ def open_dataset( path: str | os.PathLike[Any] | BufferedIOBase | AbstractDataStore, data_var: Optional[str] = None, - add_bounds: List[CFAxisKey] | None | Literal[False] = ["X", "Y"], + add_bounds: List[CFAxisKey] | None | bool = ["X", "Y"], decode_times: bool = True, center_times: bool = False, lon_orient: Optional[Tuple[float, float]] = None, @@ -54,6 +54,10 @@ def open_dataset( ) -> xr.Dataset: """Wraps ``xarray.open_dataset()`` with post-processing options. + .. deprecated:: v0.6.0 + ``add_bounds`` boolean arguments (True/False) are being deprecated. + Please use either a list (e.g., ["X", "Y"]) to specify axes or ``None``. + Parameters ---------- path : str, Path, file-like or DataStore @@ -65,10 +69,10 @@ def open_dataset( data_var: Optional[str], optional The key of the non-bounds data variable to keep in the Dataset, alongside any existing bounds data variables, by default None. - add_bounds: List[CFAxisKey] | None | Literal[False] - List of CF axes to try to add bounds for (if missing), default - ["X", "Y"]. Set to ``None`` or ``False`` to not try to add any missing - bounds. Please note that bounds are required for many xCDAT features. + add_bounds: List[CFAxisKey] | None | bool + List of CF axes to try to add bounds for (if missing), by default + ["X", "Y"]. Set to None to not add any missing bounds. Please note that + bounds are required for many xCDAT features. * This parameter calls :py:func:`xarray.Dataset.bounds.add_missing_bounds` * Supported CF axes include "X", "Y", "Z", and "T" @@ -141,6 +145,10 @@ def open_mfdataset( ) -> xr.Dataset: """Wraps ``xarray.open_mfdataset()`` with post-processing options. + .. deprecated:: v0.6.0 + ``add_bounds`` boolean arguments (True/False) are being deprecated. + Please use either a list (e.g., ["X", "Y"]) to specify axes or ``None``. + Parameters ---------- paths : str | NestedSequence[str | os.PathLike] @@ -166,10 +174,10 @@ def open_mfdataset( CDAT (including cdms2/CDML) is in maintenance only mode and marked for end-of-life by the end of 2023. - add_bounds: List[CFAxisKey] | None | Literal[False] - List of CF axes to try to add bounds for (if missing), default - ["X", "Y"]. Set to ``None`` or ``False`` to not try to add any missing - bounds. Please note that bounds are required for many xCDAT features. + add_bounds: List[CFAxisKey] | None | bool + List of CF axes to try to add bounds for (if missing), by default + ["X", "Y"]. Set to None to not add any missing bounds. Please note that + bounds are required for many xCDAT features. * This parameter calls :py:func:`xarray.Dataset.bounds.add_missing_bounds` * Supported CF axes include "X", "Y", "Z", and "T" @@ -550,11 +558,15 @@ def _postprocess_dataset( dataset: xr.Dataset, data_var: Optional[str] = None, center_times: bool = False, - add_bounds: List[CFAxisKey] | None | Literal[False] = ["X", "Y"], + add_bounds: List[CFAxisKey] | None | bool = ["X", "Y"], lon_orient: Optional[Tuple[float, float]] = None, ) -> xr.Dataset: """Post-processes a Dataset object. + .. deprecated:: v0.6.0 + ``add_bounds`` boolean arguments (True/False) are being deprecated. + Please use either a list (e.g., ["X", "Y"]) to specify axes or ``None``. + Parameters ---------- dataset : xr.Dataset @@ -565,10 +577,9 @@ def _postprocess_dataset( If True, center time coordinates using the midpoint between its upper and lower bounds. Otherwise, use the provided time coordinates, by default False. - add_bounds: List[CFAxisKey] | None | Literal[False] + add_bounds: List[CFAxisKey] | None | bool List of CF axes to try to add bounds for (if missing), default - ["X", "Y"]. Set to ``None`` or ``False`` to not try to add any missing - bounds. + ["X", "Y"]. Set to None to not add any missing bounds. * This parameter simply calls :py:func:`xarray.Dataset.bounds.add_missing_bounds` * Supported CF axes include "X", "Y", "Z", and "T" @@ -607,7 +618,27 @@ def _postprocess_dataset( if center_times: ds = center_times_func(dataset) - if add_bounds is not None and add_bounds is not False: + # TODO: Boolean (`True`/`False`) will be deprecated after v0.6.0. + if add_bounds is True: + add_bounds = ["X", "Y"] + warnings.warn( + ( + "`add_bounds=True` will be deprecated after v0.6.0. Please use a list " + "of axis strings instead (e.g., `add_bounds=['X', 'Y'])." + ), + DeprecationWarning, + stacklevel=2, + ) + elif add_bounds is False: + add_bounds = None + warnings.warn( + "`add_bounds=False` will be deprecated after v0.6.0. Please use " + "`add_bounds=None`` instead.", + DeprecationWarning, + stacklevel=2, + ) + + if add_bounds is not None: ds = ds.bounds.add_missing_bounds(axes=add_bounds) if lon_orient is not None: From e8ab146370f6e9b7f49cd03d2dd3e2d41e7a0461 Mon Sep 17 00:00:00 2001 From: tomvothecoder Date: Thu, 14 Sep 2023 14:47:56 -0700 Subject: [PATCH 2/5] Add tests --- tests/test_dataset.py | 40 ++++++++++++++++++++++++++++++++++++++++ xcdat/dataset.py | 6 ++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/tests/test_dataset.py b/tests/test_dataset.py index 674e6c8d..cf3eb8bd 100644 --- a/tests/test_dataset.py +++ b/tests/test_dataset.py @@ -324,6 +324,46 @@ def test_keeps_specified_var_and_preserves_bounds(self): assert result.identical(expected) + def test_raises_deprecation_warning_when_passing_add_bounds_true(self): + ds_no_bounds = generate_dataset( + decode_times=True, cf_compliant=True, has_bounds=False + ) + ds_no_bounds.to_netcdf(self.file_path) + + with warnings.catch_warnings(record=True) as w: + result = open_dataset(self.file_path, add_bounds=True) + + assert len(w) == 1 + assert issubclass(w[0].category, DeprecationWarning) + assert str(w[0].message) == ( + "`add_bounds=True` will be deprecated after v0.6.0. Please use a list " + "of axis strings instead (e.g., `add_bounds=['X', 'Y'])." + ) + + expected = generate_dataset( + decode_times=True, cf_compliant=True, has_bounds=True + ) + + assert result.identical(expected) + + def test_raises_deprecation_warning_when_passing_add_bounds_false(self): + ds_no_bounds = generate_dataset( + decode_times=True, cf_compliant=True, has_bounds=False + ) + ds_no_bounds.to_netcdf(self.file_path) + + with warnings.catch_warnings(record=True) as w: + result = open_dataset(self.file_path, add_bounds=True) + + assert len(w) == 1 + assert issubclass(w[0].category, DeprecationWarning) + assert str(w[0].message) == ( + "`add_bounds=False` will be deprecated after v0.6.0. Please use " + "`add_bounds=None`` instead." + ) + + assert result.identical(ds_no_bounds) + class TestOpenMfDataset: @pytest.fixture(autouse=True) diff --git a/xcdat/dataset.py b/xcdat/dataset.py index dd321bb1..fd42790d 100644 --- a/xcdat/dataset.py +++ b/xcdat/dataset.py @@ -632,8 +632,10 @@ def _postprocess_dataset( elif add_bounds is False: add_bounds = None warnings.warn( - "`add_bounds=False` will be deprecated after v0.6.0. Please use " - "`add_bounds=None`` instead.", + ( + "`add_bounds=False` will be deprecated after v0.6.0. Please use " + "`add_bounds=None`` instead." + ), DeprecationWarning, stacklevel=2, ) From d0d1f528eb7f7845a59b1fb9ef43fedb4fe8c49d Mon Sep 17 00:00:00 2001 From: Tom Vo Date: Thu, 14 Sep 2023 14:59:16 -0700 Subject: [PATCH 3/5] Update workspace settings and tests --- .vscode/xcdat.code-workspace | 12 ++++++------ tests/test_dataset.py | 4 +++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/.vscode/xcdat.code-workspace b/.vscode/xcdat.code-workspace index 42ebefdf..bdfe5127 100644 --- a/.vscode/xcdat.code-workspace +++ b/.vscode/xcdat.code-workspace @@ -21,17 +21,17 @@ "editor.rulers": [80, 88, 120], "editor.wordWrap": "wordWrapColumn", "editor.wordWrapColumn": 120, - "editor.defaultFormatter": "ms-python.python" + "editor.defaultFormatter": "ms-python.black-formatter" }, + "black-formatter.importStrategy": "fromEnvironment", // Code Formatting and Linting // --------------------------- - "python.formatting.provider": "black", - "python.linting.flake8Enabled": true, - "python.linting.flake8Args": ["--config=setup.cfg"], + "flake8.args": ["--config=setup.cfg"], + "flake8.importStrategy": "fromEnvironment", // Type checking // --------------------------- - "python.linting.mypyEnabled": true, - "python.linting.mypyArgs": ["--config=setup.cfg"], + "mypy-type-checker.args": ["--config=pyproject.toml"], + "mypy-type-checker.importStrategy": "fromEnvironment", // Testing // --------------------------- "python.testing.unittestEnabled": false, diff --git a/tests/test_dataset.py b/tests/test_dataset.py index cf3eb8bd..0875f19a 100644 --- a/tests/test_dataset.py +++ b/tests/test_dataset.py @@ -343,6 +343,8 @@ def test_raises_deprecation_warning_when_passing_add_bounds_true(self): expected = generate_dataset( decode_times=True, cf_compliant=True, has_bounds=True ) + expected = expected.drop_vars("time_bnds") + del expected["time"].attrs["bounds"] assert result.identical(expected) @@ -353,7 +355,7 @@ def test_raises_deprecation_warning_when_passing_add_bounds_false(self): ds_no_bounds.to_netcdf(self.file_path) with warnings.catch_warnings(record=True) as w: - result = open_dataset(self.file_path, add_bounds=True) + result = open_dataset(self.file_path, add_bounds=False) assert len(w) == 1 assert issubclass(w[0].category, DeprecationWarning) From 0e7f10314ca4fa48f32f7d33432267facfcbd884 Mon Sep 17 00:00:00 2001 From: Tom Vo Date: Thu, 14 Sep 2023 15:02:03 -0700 Subject: [PATCH 4/5] Fix deprecation warning string --- xcdat/dataset.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xcdat/dataset.py b/xcdat/dataset.py index fd42790d..147d9db7 100644 --- a/xcdat/dataset.py +++ b/xcdat/dataset.py @@ -624,7 +624,7 @@ def _postprocess_dataset( warnings.warn( ( "`add_bounds=True` will be deprecated after v0.6.0. Please use a list " - "of axis strings instead (e.g., `add_bounds=['X', 'Y'])." + "of axis strings instead (e.g., `add_bounds=['X', 'Y']`)." ), DeprecationWarning, stacklevel=2, @@ -634,7 +634,7 @@ def _postprocess_dataset( warnings.warn( ( "`add_bounds=False` will be deprecated after v0.6.0. Please use " - "`add_bounds=None`` instead." + "`add_bounds=None` instead." ), DeprecationWarning, stacklevel=2, From 73e685133f9a3a7ca398bee7a2a96152aefb138d Mon Sep 17 00:00:00 2001 From: Tom Vo Date: Thu, 14 Sep 2023 15:08:38 -0700 Subject: [PATCH 5/5] Fix tests --- tests/test_dataset.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_dataset.py b/tests/test_dataset.py index 0875f19a..74b5edcc 100644 --- a/tests/test_dataset.py +++ b/tests/test_dataset.py @@ -337,7 +337,7 @@ def test_raises_deprecation_warning_when_passing_add_bounds_true(self): assert issubclass(w[0].category, DeprecationWarning) assert str(w[0].message) == ( "`add_bounds=True` will be deprecated after v0.6.0. Please use a list " - "of axis strings instead (e.g., `add_bounds=['X', 'Y'])." + "of axis strings instead (e.g., `add_bounds=['X', 'Y']`)." ) expected = generate_dataset( @@ -361,7 +361,7 @@ def test_raises_deprecation_warning_when_passing_add_bounds_false(self): assert issubclass(w[0].category, DeprecationWarning) assert str(w[0].message) == ( "`add_bounds=False` will be deprecated after v0.6.0. Please use " - "`add_bounds=None`` instead." + "`add_bounds=None` instead." ) assert result.identical(ds_no_bounds)