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
5 changes: 4 additions & 1 deletion doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ Breaking changes

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

- Following pandas, the `closed` parameters of :py:func:`cftime_range` and
:py:func:`date_range` are deprecated in favor of the `inclusive` parameters,
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
100 changes: 86 additions & 14 deletions xarray/coding/cftime_offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
import re
from datetime import datetime, timedelta
from functools import partial
from typing import ClassVar
from typing import TYPE_CHECKING, ClassVar

import numpy as np
import pandas as pd
Expand All @@ -57,14 +57,19 @@
format_cftime_datetime,
)
from xarray.core.common import _contains_datetime_like_objects, is_np_datetime_like
from xarray.core.pdcompat import count_not_none
from xarray.core.pdcompat import NoDefault, count_not_none, no_default
from xarray.core.utils import emit_user_level_warning

try:
import cftime
except ImportError:
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 @@ -849,14 +854,49 @@ 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, inclusive):
"""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:
inclusive = _translate_closed_to_inclusive(closed)
elif inclusive is None:
inclusive = "both"
return inclusive


def cftime_range(
start=None,
end=None,
periods=None,
freq="D",
normalize=False,
name=None,
closed=None,
closed: NoDefault | SideOptions = no_default,
inclusive: None | InclusiveOptions = None,
calendar="standard",
):
"""Return a fixed frequency CFTimeIndex.
Expand All @@ -875,9 +915,20 @@ def cftime_range(
Normalize start/end dates to midnight before generating date range.
name : str, default: None
Name of the resulting index
closed : {"left", "right"} or None, default: None
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.01.1
dcherian marked this conversation as resolved.
Show resolved Hide resolved
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
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.

Expand Down Expand Up @@ -1047,18 +1098,25 @@ def cftime_range(
offset = to_offset(freq)
dates = np.array(list(_generate_range(start, end, periods, offset)))

left_closed = False
right_closed = False
inclusive = _infer_inclusive(closed, inclusive)

if closed is None:
if inclusive == "neither":
left_closed = False
right_closed = False
elif inclusive == "left":
left_closed = True
right_closed = False
elif inclusive == "right":
left_closed = False
right_closed = True
elif closed == "left":
elif inclusive == "both":
left_closed = True
elif closed == "right":
right_closed = True
else:
raise ValueError("Closed must be either 'left', 'right' or None")
raise ValueError(
f"Argument `inclusive` must be either 'both', 'neither', "
f"'left', 'right', or None. Got {inclusive}."
)

if not left_closed and len(dates) and start is not None and dates[0] == start:
dates = dates[1:]
Expand All @@ -1076,7 +1134,8 @@ def date_range(
tz=None,
normalize=False,
name=None,
closed=None,
closed: NoDefault | SideOptions = no_default,
inclusive: None | InclusiveOptions = None,
calendar="standard",
use_cftime=None,
):
Expand All @@ -1103,9 +1162,20 @@ def date_range(
Normalize start/end dates to midnight before generating date range.
name : str, default: None
Name of the resulting index
closed : {"left", "right"} or None, default: None
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.01.1
dcherian marked this conversation as resolved.
Show resolved Hide resolved
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
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
Expand All @@ -1129,6 +1199,8 @@ 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 All @@ -1139,7 +1211,7 @@ def date_range(
tz=tz,
normalize=normalize,
name=name,
closed=closed,
inclusive=inclusive,
dcherian marked this conversation as resolved.
Show resolved Hide resolved
)
except pd.errors.OutOfBoundsDatetime as err:
if use_cftime is False:
Expand All @@ -1158,7 +1230,7 @@ def date_range(
freq=freq,
normalize=normalize,
name=name,
closed=closed,
inclusive=inclusive,
calendar=calendar,
)

Expand Down
26 changes: 26 additions & 0 deletions xarray/core/pdcompat.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,36 @@
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
from __future__ import annotations

from enum import Enum
from typing import Literal


def count_not_none(*args) -> int:
"""Compute the number of non-None arguments.

Copied from pandas.core.common.count_not_none (not part of the public API)
"""
return sum(arg is not None for arg in args)


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

See also:
- pandas-dev/pandas#30788
- pandas-dev/pandas#40684
- pandas-dev/pandas#40715
- pandas-dev/pandas#47045
"""

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
1 change: 1 addition & 0 deletions xarray/core/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading