Skip to content

Commit

Permalink
Add documentation of deprecation
Browse files Browse the repository at this point in the history
  • Loading branch information
spencerkclark committed Jan 23, 2023
1 parent 11a9e0d commit 3f86d33
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 20 deletions.
5 changes: 4 additions & 1 deletion doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ Breaking changes

Deprecations
~~~~~~~~~~~~

- Following pandas, the `closed` arguments of :py:func:`cftime_range` and
:py:func:`date_range` are deprecated in favor of the `inclusive` arguments,
and will be removed in a future version of xarray (:issue:`6985`:,
:pull:`7373`). By `Spencer Clark <https://github.com/spencerkclark>`_.

Bug fixes
~~~~~~~~~
Expand Down
53 changes: 40 additions & 13 deletions xarray/coding/cftime_offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
from datetime import datetime, timedelta
from enum import Enum
from functools import partial
from typing import ClassVar
from typing import TYPE_CHECKING, ClassVar, Literal

import numpy as np
import pandas as pd
Expand All @@ -67,6 +67,10 @@
cftime = None


if TYPE_CHECKING:
from xarray.core.types import InclusiveOptions, SideOptions


def get_date_type(calendar, use_cftime=True):
"""Return the cftime date type for a given calendar name."""
if cftime is None:
Expand Down Expand Up @@ -852,15 +856,24 @@ def _generate_range(start, end, periods, offset):


class _NoDefault(Enum):
"""Used by pandas to specify a default value for a deprecated argument."""
"""Used by pandas to specify a default value for a deprecated argument.
Copied from pandas._libs.lib._NoDefault.
"""

no_default = "NO_DEFAULT"

def __repr__(self) -> str:
return "<no_default>"


no_default = (
_NoDefault.no_default
) # Sentinel indicating the default value following pandas
NoDefault = Literal[_NoDefault.no_default] # For typing following pandas


def _translate_closed_to_inclusive(closed):
"""Follows code added in pandas #43504."""
emit_user_level_warning(
"Following pandas, the `closed` argument is deprecated in "
"favor of the `inclusive` argument, and will be removed in "
Expand All @@ -880,12 +893,13 @@ def _translate_closed_to_inclusive(closed):


def _infer_inclusive(closed, inclusive):
if closed is not _NoDefault and inclusive is not None:
"""Follows code added in pandas #43504."""
if closed is not no_default and inclusive is not None:
raise ValueError(
"Deprecated argument `closed` cannot be passed if "
"argument `inclusive` is not None."
"Following pandas, deprecated argument `closed` cannot be "
"passed if argument `inclusive` is not None."
)
if closed is not _NoDefault:
if closed is not no_default:
inclusive = _translate_closed_to_inclusive(closed)
elif inclusive is None:
inclusive = "both"
Expand All @@ -899,8 +913,8 @@ def cftime_range(
freq="D",
normalize=False,
name=None,
closed=_NoDefault,
inclusive=None,
closed: NoDefault | SideOptions = no_default,
inclusive: None | InclusiveOptions = None,
calendar="standard",
):
"""Return a fixed frequency CFTimeIndex.
Expand All @@ -922,8 +936,15 @@ def cftime_range(
closed : {"left", "right"} or None, default: _NoDefault
Make the interval closed with respect to the given frequency to the
"left", "right", or both sides (None).
.. deprecated:: 2023.01.1
Following pandas, the `closed` argument is deprecated in favor
of the `inclusive` argument, and will be removed in a future
version of xarray.
inclusive : {None, "both", "neither", "left", "right"}, default None
Include boundaries; Whether to set each bound as closed or open.
Include boundaries; whether to set each bound as closed or open.
.. versionadded:: 2023.01.1
calendar : str, default: "standard"
Calendar type for the datetimes.
Expand All @@ -938,7 +959,6 @@ def cftime_range(
features of ``pandas.date_range`` (e.g. specifying how the index is
``closed`` on either side, or whether or not to ``normalize`` the start and
end bounds); however, there are some notable exceptions:
- You cannot specify a ``tz`` (time zone) argument.
- Start or end dates specified as partial-datetime strings must use the
`ISO-8601 format <https://en.wikipedia.org/wiki/ISO_8601>`_.
Expand Down Expand Up @@ -1129,8 +1149,8 @@ def date_range(
tz=None,
normalize=False,
name=None,
closed=_NoDefault,
inclusive=None,
closed: NoDefault | SideOptions = no_default,
inclusive: None | InclusiveOptions = None,
calendar="standard",
use_cftime=None,
):
Expand Down Expand Up @@ -1160,8 +1180,15 @@ def date_range(
closed : {"left", "right"} or None, default: _NoDefault
Make the interval closed with respect to the given frequency to the
"left", "right", or both sides (None).
.. deprecated:: 2023.01.1
Following pandas, the `closed` argument is deprecated in favor
of the `inclusive` argument, and will be removed in a future
version of xarray.
inclusive : {None, "both", "neither", "left", "right"}, default None
Include boundaries; Whether to set each bound as closed or open.
Include boundaries; whether to set each bound as closed or open.
.. versionadded:: 2023.01.1
calendar : str, default: "standard"
Calendar type for the datetimes.
use_cftime : boolean, optional
Expand Down
1 change: 1 addition & 0 deletions xarray/core/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ def dtype(self) -> np.dtype:

CoarsenBoundaryOptions = Literal["exact", "trim", "pad"]
SideOptions = Literal["left", "right"]
InclusiveOptions = Literal["both", "neither", "left", "right"]

ScaleOptions = Literal["linear", "symlog", "log", "logit", None]
HueStyleOptions = Literal["continuous", "discrete", None]
Expand Down
10 changes: 5 additions & 5 deletions xarray/tests/test_cftime_offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -1411,16 +1411,16 @@ def as_timedelta_not_implemented_error():


@pytest.mark.parametrize("function", [cftime_range, date_range])
def test_cftime_or_date_range_closed_and_inclusive_error(function):
def test_cftime_or_date_range_closed_and_inclusive_error(function) -> None:
if function == cftime_range and not has_cftime:
pytest.skip("requires cftime")

with pytest.raises(ValueError, match="Deprecated argument `closed`"):
with pytest.raises(ValueError, match="Following pandas, deprecated"):
function("2000", periods=3, closed=None, inclusive="right")


@pytest.mark.parametrize("function", [cftime_range, date_range])
def test_cftime_or_date_range_invalid_closed_value(function):
def test_cftime_or_date_range_invalid_closed_value(function) -> None:
if function == cftime_range and not has_cftime:
pytest.skip("requires cftime")

Expand All @@ -1432,7 +1432,7 @@ def test_cftime_or_date_range_invalid_closed_value(function):
@pytest.mark.parametrize(
("closed", "inclusive"), [(None, "both"), ("left", "left"), ("right", "right")]
)
def test_cftime_or_date_range_closed(function, closed, inclusive):
def test_cftime_or_date_range_closed(function, closed, inclusive) -> None:
if function == cftime_range and not has_cftime:
pytest.skip("requires cftime")

Expand All @@ -1445,7 +1445,7 @@ def test_cftime_or_date_range_closed(function, closed, inclusive):


@pytest.mark.parametrize("function", [cftime_range, date_range])
def test_cftime_or_date_range_inclusive_None(function):
def test_cftime_or_date_range_inclusive_None(function) -> None:
if function == cftime_range and not has_cftime:
pytest.skip("requires cftime")

Expand Down
9 changes: 8 additions & 1 deletion xarray/tests/test_cftimeindex_resample.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import datetime
from typing import TypedDict

import numpy as np
import pandas as pd
Expand Down Expand Up @@ -189,6 +190,12 @@ def test_calendars(calendar) -> None:
xr.testing.assert_identical(da_cftime, da_datetime)


class DateRangeKwargs(TypedDict):
start: str
periods: int
freq: str


@pytest.mark.parametrize("closed", ["left", "right"])
@pytest.mark.parametrize(
"origin",
Expand All @@ -198,7 +205,7 @@ def test_calendars(calendar) -> None:
def test_origin(closed, origin) -> None:
initial_freq, resample_freq = ("3H", "9H")
start = "1969-12-31T12:07:01"
index_kwargs = dict(start=start, periods=12, freq=initial_freq)
index_kwargs: DateRangeKwargs = dict(start=start, periods=12, freq=initial_freq)
datetime_index = pd.date_range(**index_kwargs)
cftime_index = xr.cftime_range(**index_kwargs)
da_datetimeindex = da(datetime_index)
Expand Down

0 comments on commit 3f86d33

Please sign in to comment.