Skip to content
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

Add deprecation warnings for add_bounds boolean args #548

Merged
merged 5 commits into from
Sep 14, 2023
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
12 changes: 6 additions & 6 deletions .vscode/xcdat.code-workspace
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
42 changes: 42 additions & 0 deletions tests/test_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,48 @@ 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
)
expected = expected.drop_vars("time_bnds")
del expected["time"].attrs["bounds"]

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=False)

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)
Expand Down
61 changes: 47 additions & 14 deletions xcdat/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,18 @@
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,
**kwargs: Dict[str, Any],
) -> 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
Expand All @@ -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"
Expand Down Expand Up @@ -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]
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -607,7 +618,29 @@ 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:
Expand Down
Loading