diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 26bd72b0727..63ac491e61e 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -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 `_. Bug fixes ~~~~~~~~~ diff --git a/xarray/coding/cftime_offsets.py b/xarray/coding/cftime_offsets.py index 801fbbf7052..bc3e3545892 100644 --- a/xarray/coding/cftime_offsets.py +++ b/xarray/coding/cftime_offsets.py @@ -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 @@ -57,7 +57,8 @@ 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 @@ -65,6 +66,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: @@ -849,6 +854,40 @@ 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, @@ -856,7 +895,8 @@ def cftime_range( freq="D", normalize=False, name=None, - closed=None, + closed: NoDefault | SideOptions = no_default, + inclusive: None | InclusiveOptions = None, calendar="standard", ): """Return a fixed frequency CFTimeIndex. @@ -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.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 + Include boundaries; whether to set each bound as closed or open. + + .. versionadded:: 2023.02.0 + calendar : str, default: "standard" Calendar type for the datetimes. @@ -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:] @@ -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, ): @@ -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.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 + 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 @@ -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( @@ -1139,7 +1211,7 @@ def date_range( tz=tz, normalize=normalize, name=name, - closed=closed, + inclusive=inclusive, ) except pd.errors.OutOfBoundsDatetime as err: if use_cftime is False: @@ -1158,7 +1230,7 @@ def date_range( freq=freq, normalize=normalize, name=name, - closed=closed, + inclusive=inclusive, calendar=calendar, ) diff --git a/xarray/core/pdcompat.py b/xarray/core/pdcompat.py index 7a4b3c405ae..018bb19b871 100644 --- a/xarray/core/pdcompat.py +++ b/xarray/core/pdcompat.py @@ -35,6 +35,9 @@ # 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. @@ -42,3 +45,26 @@ def count_not_none(*args) -> int: 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 = ( + _NoDefault.no_default +) # Sentinel indicating the default value following pandas +NoDefault = Literal[_NoDefault.no_default] # For typing following pandas diff --git a/xarray/core/types.py b/xarray/core/types.py index fc3c6712be2..7149443c0c7 100644 --- a/xarray/core/types.py +++ b/xarray/core/types.py @@ -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] diff --git a/xarray/tests/test_cftime_offsets.py b/xarray/tests/test_cftime_offsets.py index c981015521b..6b628c15488 100644 --- a/xarray/tests/test_cftime_offsets.py +++ b/xarray/tests/test_cftime_offsets.py @@ -33,7 +33,7 @@ ) from xarray.coding.frequencies import infer_freq from xarray.core.dataarray import DataArray -from xarray.tests import _CFTIME_CALENDARS, requires_cftime +from xarray.tests import _CFTIME_CALENDARS, has_cftime, requires_cftime cftime = pytest.importorskip("cftime") @@ -1022,7 +1022,25 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg "0001-01-04", None, "D", + "neither", + 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", None, + "D", + "both", False, [(1, 1, 1), (1, 1, 2), (1, 1, 3), (1, 1, 4)], ), @@ -1049,7 +1067,7 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg "0001-01-04", None, "D", - None, + "both", False, [(1, 1, 1, 1), (1, 1, 2, 1), (1, 1, 3, 1)], ), @@ -1058,7 +1076,7 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg "0001-01-04", None, "D", - None, + "both", False, [(1, 1, 1, 1), (1, 1, 2, 1), (1, 1, 3, 1)], ), @@ -1067,7 +1085,7 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg "0001-01-04", None, "D", - None, + "both", True, [(1, 1, 1), (1, 1, 2), (1, 1, 3), (1, 1, 4)], ), @@ -1076,7 +1094,7 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg None, 4, "D", - None, + "both", False, [(1, 1, 1), (1, 1, 2), (1, 1, 3), (1, 1, 4)], ), @@ -1085,7 +1103,7 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg "0001-01-04", 4, "D", - None, + "both", False, [(1, 1, 1), (1, 1, 2), (1, 1, 3), (1, 1, 4)], ), @@ -1094,7 +1112,7 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg "0001-01-04", None, "D", - None, + "both", False, [(1, 1, 1), (1, 1, 2), (1, 1, 3), (1, 1, 4)], ), @@ -1103,7 +1121,7 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg (1, 1, 4), None, "D", - None, + "both", False, [(1, 1, 1), (1, 1, 2), (1, 1, 3), (1, 1, 4)], ), @@ -1112,17 +1130,17 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg "0011-02-01", None, "3AS-JUN", - None, + "both", False, [(1, 6, 1), (4, 6, 1), (7, 6, 1), (10, 6, 1)], ), - ("0001-01-04", "0001-01-01", None, "D", None, False, []), + ("0001-01-04", "0001-01-01", None, "D", "both", False, []), ( "0010", None, 4, YearBegin(n=-2), - None, + "both", False, [(10, 1, 1), (8, 1, 1), (6, 1, 1), (4, 1, 1)], ), @@ -1131,7 +1149,7 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg "0001-01-04", 4, None, - None, + "both", False, [(1, 1, 1), (1, 1, 2), (1, 1, 3), (1, 1, 4)], ), @@ -1140,7 +1158,7 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg None, 4, "3QS-JUN", - None, + "both", False, [(1, 6, 1), (2, 3, 1), (2, 12, 1), (3, 9, 1)], ), @@ -1148,12 +1166,12 @@ def test_rollback(calendar, offset, initial_date_args, partial_expected_date_arg @pytest.mark.parametrize( - ("start", "end", "periods", "freq", "closed", "normalize", "expected_date_args"), + ("start", "end", "periods", "freq", "inclusive", "normalize", "expected_date_args"), _CFTIME_RANGE_TESTS, ids=_id_func, ) def test_cftime_range( - start, end, periods, freq, closed, normalize, calendar, expected_date_args + start, end, periods, freq, inclusive, normalize, calendar, expected_date_args ): date_type = get_date_type(calendar) expected_dates = [date_type(*args) for args in expected_date_args] @@ -1168,7 +1186,7 @@ def test_cftime_range( end=end, periods=periods, freq=freq, - closed=closed, + inclusive=inclusive, normalize=normalize, calendar=calendar, ) @@ -1390,3 +1408,47 @@ def as_timedelta_not_implemented_error(): tick = Tick() with pytest.raises(NotImplementedError): tick.as_timedelta() + + +@pytest.mark.parametrize("function", [cftime_range, date_range]) +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="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) -> None: + if function == cftime_range and not has_cftime: + pytest.skip("requires cftime") + + with pytest.raises(ValueError, match="Argument `closed` must be"): + function("2000", periods=3, closed="foo") + + +@pytest.mark.parametrize("function", [cftime_range, date_range]) +@pytest.mark.parametrize( + ("closed", "inclusive"), [(None, "both"), ("left", "left"), ("right", "right")] +) +def test_cftime_or_date_range_closed(function, closed, inclusive) -> None: + if function == cftime_range and not has_cftime: + pytest.skip("requires cftime") + + 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: + pytest.skip("requires cftime") + + result_None = function("2000-01-01", "2000-01-04") + result_both = function("2000-01-01", "2000-01-04", inclusive="both") + np.testing.assert_equal(result_None.values, result_both.values) diff --git a/xarray/tests/test_cftimeindex_resample.py b/xarray/tests/test_cftimeindex_resample.py index e780421e09e..5f818b7663d 100644 --- a/xarray/tests/test_cftimeindex_resample.py +++ b/xarray/tests/test_cftimeindex_resample.py @@ -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)