Skip to content

Commit

Permalink
finalize deprecation of "closed"-parameter (#9882)
Browse files Browse the repository at this point in the history 
* finalize deprecation of "closed" to "inclusive" in date_range and cftime_range

* add whats-new.rst entry

* fix tests

* fix test

* remove stale function
  • Loading branch information
@kmuehlbauer
kmuehlbauer authored Dec 13, 2024
1 parent 49502fc commit b7e6036
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 117 deletions.
4 changes: 3 additions & 1 deletion doc/whats-new.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ Breaking changes

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

- Finalize deprecation of ``closed`` parameters of :py:func:`cftime_range` and
:py:func:`date_range` (:pull:`9882`).
By `Kai Mühlbauer <https://github.com/kmuehlbauer>`_.

Bug fixes
~~~~~~~~~
Expand Down
73 changes: 5 additions & 68 deletions xarray/coding/cftime_offsets.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,13 @@
)
from xarray.core.common import _contains_datetime_like_objects, is_np_datetime_like
from xarray.core.pdcompat import (
NoDefault,
count_not_none,
nanosecond_precision_timestamp,
no_default,
)
from xarray.core.utils import attempt_import, emit_user_level_warning

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


DayOption: TypeAlias = Literal["start", "end"]
Expand Down Expand Up @@ -943,51 +941,14 @@ def _generate_range(start, end, periods, offset):
current = next_date


def _translate_closed_to_inclusive(closed):
"""Follows code added in pandas #43504."""
emit_user_level_warning(
"Following pandas, the `closed` parameter is deprecated in "
"favor of the `inclusive` parameter, and will be removed in "
"a future version of xarray.",
FutureWarning,
)
if closed is None:
inclusive = "both"
elif closed in ("left", "right"):
inclusive = closed
else:
raise ValueError(
f"Argument `closed` must be either 'left', 'right', or None. "
f"Got {closed!r}."
)
return inclusive


def _infer_inclusive(
closed: NoDefault | SideOptions, inclusive: InclusiveOptions | None
) -> InclusiveOptions:
"""Follows code added in pandas #43504."""
if closed is not no_default and inclusive is not None:
raise ValueError(
"Following pandas, deprecated argument `closed` cannot be "
"passed if argument `inclusive` is not None."
)
if closed is not no_default:
return _translate_closed_to_inclusive(closed)
if inclusive is None:
return "both"
return inclusive


def cftime_range(
start=None,
end=None,
periods=None,
freq=None,
normalize=False,
name=None,
closed: NoDefault | SideOptions = no_default,
inclusive: None | InclusiveOptions = None,
inclusive: InclusiveOptions = "both",
calendar="standard",
) -> CFTimeIndex:
"""Return a fixed frequency CFTimeIndex.
Expand All @@ -1006,16 +967,7 @@ def cftime_range(
Normalize start/end dates to midnight before generating date range.
name : str, default: None
Name of the resulting index
closed : {None, "left", "right"}, default: "NO_DEFAULT"
Make the interval closed with respect to the given frequency to the
"left", "right", or both sides (None).
.. deprecated:: 2023.02.0
Following pandas, the ``closed`` parameter is deprecated in favor
of the ``inclusive`` parameter, and will be removed in a future
version of xarray.
inclusive : {None, "both", "neither", "left", "right"}, default None
inclusive : {"both", "neither", "left", "right"}, default "both"
Include boundaries; whether to set each bound as closed or open.
.. versionadded:: 2023.02.0
Expand Down Expand Up @@ -1193,8 +1145,6 @@ def cftime_range(
offset = to_offset(freq)
dates = np.array(list(_generate_range(start, end, periods, offset)))

inclusive = _infer_inclusive(closed, inclusive)

if inclusive == "neither":
left_closed = False
right_closed = False
Expand Down Expand Up @@ -1229,8 +1179,7 @@ def date_range(
tz=None,
normalize=False,
name=None,
closed: NoDefault | SideOptions = no_default,
inclusive: None | InclusiveOptions = None,
inclusive: InclusiveOptions = "both",
calendar="standard",
use_cftime=None,
):
Expand All @@ -1257,20 +1206,10 @@ def date_range(
Normalize start/end dates to midnight before generating date range.
name : str, default: None
Name of the resulting index
closed : {None, "left", "right"}, default: "NO_DEFAULT"
Make the interval closed with respect to the given frequency to the
"left", "right", or both sides (None).
.. deprecated:: 2023.02.0
Following pandas, the `closed` parameter is deprecated in favor
of the `inclusive` parameter, and will be removed in a future
version of xarray.
inclusive : {None, "both", "neither", "left", "right"}, default: None
inclusive : {"both", "neither", "left", "right"}, default: "both"
Include boundaries; whether to set each bound as closed or open.
.. versionadded:: 2023.02.0
calendar : str, default: "standard"
Calendar type for the datetimes.
use_cftime : boolean, optional
Expand All @@ -1294,8 +1233,6 @@ def date_range(
if tz is not None:
use_cftime = False

inclusive = _infer_inclusive(closed, inclusive)

if _is_standard_calendar(calendar) and use_cftime is not True:
try:
return pd.date_range(
Expand Down
48 changes: 0 additions & 48 deletions xarray/tests/test_cftime_offsets.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1057,15 +1057,6 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg
False,
[(1, 1, 2), (1, 1, 3)],
),
(
"0001-01-01",
"0001-01-04",
None,
"D",
None,
False,
[(1, 1, 1), (1, 1, 2), (1, 1, 3), (1, 1, 4)],
),
(
"0001-01-01",
"0001-01-04",
Expand Down Expand Up @@ -1294,13 +1285,6 @@ def test_invalid_cftime_range_inputs(
cftime_range(start, end, periods, freq, inclusive=inclusive) # type: ignore[arg-type]


def test_invalid_cftime_arg() -> None:
with pytest.warns(
FutureWarning, match="Following pandas, the `closed` parameter is deprecated"
):
cftime_range("2000", "2001", None, "YE", closed="left")


_CALENDAR_SPECIFIC_MONTH_END_TESTS = [
("noleap", [(2, 28), (4, 30), (6, 30), (8, 31), (10, 31), (12, 31)]),
("all_leap", [(2, 29), (4, 30), (6, 30), (8, 31), (10, 31), (12, 31)]),
Expand Down Expand Up @@ -1534,15 +1518,6 @@ def as_timedelta_not_implemented_error():
tick.as_timedelta()


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

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_inclusive_value(function: Callable) -> None:
if function == cftime_range and not has_cftime:
Expand All @@ -1552,29 +1527,6 @@ def test_cftime_or_date_range_invalid_inclusive_value(function: Callable) -> Non
function("2000", periods=3, inclusive="foo")


@pytest.mark.parametrize(
"function",
[
pytest.param(cftime_range, id="cftime", marks=requires_cftime),
pytest.param(date_range, id="date"),
],
)
@pytest.mark.parametrize(
("closed", "inclusive"), [(None, "both"), ("left", "left"), ("right", "right")]
)
def test_cftime_or_date_range_closed(
function: Callable,
closed: Literal["left", "right", None],
inclusive: Literal["left", "right", "both"],
) -> None:
with pytest.warns(FutureWarning, match="Following pandas"):
result_closed = function("2000-01-01", "2000-01-04", freq="D", closed=closed)
result_inclusive = function(
"2000-01-01", "2000-01-04", freq="D", inclusive=inclusive
)
np.testing.assert_equal(result_closed.values, result_inclusive.values)


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

0 comments on commit b7e6036

Please sign in to comment.