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 inclusive argument to cftime_range and date_range and deprecate closed argument #7373

Merged
merged 9 commits into from
Feb 6, 2023
Prev Previous commit
Next Next commit
Add documentation of deprecation
spencerkclark committed Feb 4, 2023
commit 1a5f13e60f05a76dff9a17dbf0e7d042de916178
5 changes: 4 additions & 1 deletion doc/whats-new.rst
Original file line number Diff line number Diff line change
@@ -45,7 +45,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
~~~~~~~~~
53 changes: 40 additions & 13 deletions xarray/coding/cftime_offsets.py
Original file line number Diff line number Diff line change
@@ -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
@@ -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:
@@ -852,15 +856,24 @@ def _generate_range(start, end, periods, offset):


class _NoDefault(Enum):
spencerkclark marked this conversation as resolved.
Show resolved Hide resolved
"""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 "
@@ -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"
@@ -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.
@@ -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
dcherian marked this conversation as resolved.
Show resolved Hide resolved
Following pandas, the `closed` argument is deprecated in favor
of the `inclusive` argument, and will be removed in a future
version of xarray.
dcherian marked this conversation as resolved.
Show resolved Hide resolved
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
dcherian marked this conversation as resolved.
Show resolved Hide resolved
calendar : str, default: "standard"
Calendar type for the datetimes.

@@ -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>`_.
@@ -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,
):
@@ -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
dcherian marked this conversation as resolved.
Show resolved Hide resolved
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
dcherian marked this conversation as resolved.
Show resolved Hide resolved
calendar : str, default: "standard"
Calendar type for the datetimes.
use_cftime : boolean, optional
1 change: 1 addition & 0 deletions xarray/core/types.py
Original file line number Diff line number Diff line change
@@ -168,6 +168,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]
10 changes: 5 additions & 5 deletions xarray/tests/test_cftime_offsets.py
Original file line number Diff line number Diff line change
@@ -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")

@@ -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")

@@ -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")

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
@@ -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",
@@ -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)