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

finalize deprecation of "closed"-parameter #9882

Merged
merged 5 commits into from
Dec 13, 2024
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
4 changes: 3 additions & 1 deletion doc/whats-new.rst
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
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
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
Loading