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

ENH: consistency of input args for boundaries (pd.date_range) #43504

Merged
merged 10 commits into from
Oct 3, 2021
Merged
2 changes: 2 additions & 0 deletions doc/source/whatsnew/v1.4.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ Other Deprecations
- Deprecated the ``squeeze`` argument to :meth:`read_csv`, :meth:`read_table`, and :meth:`read_excel`. Users should squeeze the DataFrame afterwards with ``.squeeze("columns")`` instead. (:issue:`43242`)
- Deprecated the ``index`` argument to :class:`SparseArray` construction (:issue:`23089`)
-
- Deprecated the ``closed`` argument in :meth:`date_range` and :meth:`bdate_range` in favor of ``inclusive`` argument; In a future version passing ``closed`` will raise (:issue:`40245`)

.. ---------------------------------------------------------------------------

Expand Down Expand Up @@ -340,6 +341,7 @@ Datetimelike
^^^^^^^^^^^^
- Bug in :class:`DataFrame` constructor unnecessarily copying non-datetimelike 2D object arrays (:issue:`39272`)
- :func:`to_datetime` would silently swap ``MM/DD/YYYY`` and ``DD/MM/YYYY`` formats if the given ``dayfirst`` option could not be respected - now, a warning is raised in the case of delimited date strings (e.g. ``31-12-2012``) (:issue:`12585`)
- Bug in :meth:`date_range` and :meth:`bdate_range` do not return right bound when ``start`` = ``end`` and set is closed on one side (:issue:`43394`)
-

Timedelta
Expand Down
8 changes: 8 additions & 0 deletions pandas/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,14 @@ def keep(request):
return request.param


@pytest.fixture(params=["both", "neither", "left", "right"])
def inclusive_endpoints_fixture(request):
"""
Fixture for trying all interval 'inclusive' parameters.
"""
return request.param


@pytest.fixture(params=["left", "right", "both", "neither"])
def closed(request):
"""
Expand Down
19 changes: 12 additions & 7 deletions pandas/core/arrays/datetimes.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
)
from pandas._typing import npt
from pandas.errors import PerformanceWarning
from pandas.util._validators import validate_endpoints
from pandas.util._validators import validate_inclusive

from pandas.core.dtypes.cast import astype_dt64_to_dt64tz
from pandas.core.dtypes.common import (
Expand Down Expand Up @@ -394,7 +394,7 @@ def _generate_range(
normalize=False,
ambiguous="raise",
nonexistent="raise",
closed=None,
inclusive="both",
):

periods = dtl.validate_periods(periods)
Expand All @@ -417,7 +417,7 @@ def _generate_range(
if start is NaT or end is NaT:
raise ValueError("Neither `start` nor `end` can be NaT")

left_closed, right_closed = validate_endpoints(closed)
left_inclusive, right_inclusive = validate_inclusive(inclusive)
start, end, _normalized = _maybe_normalize_endpoints(start, end, normalize)
tz = _infer_tz_from_endpoints(start, end, tz)

Expand Down Expand Up @@ -477,10 +477,15 @@ def _generate_range(
arr = arr.astype("M8[ns]", copy=False)
index = cls._simple_new(arr, freq=None, dtype=dtype)

if not left_closed and len(index) and index[0] == start:
index = index[1:]
if not right_closed and len(index) and index[-1] == end:
index = index[:-1]
if start == end:
if not left_inclusive and not right_inclusive:
index = index[1:-1]
else:
if not left_inclusive or not right_inclusive:
if not left_inclusive and len(index) and index[0] == start:
index = index[1:]
if not right_inclusive and len(index) and index[-1] == end:
index = index[:-1]

dtype = tz_to_dtype(tz)
return cls._simple_new(index._ndarray, freq=freq, dtype=dtype)
Expand Down
47 changes: 44 additions & 3 deletions pandas/core/indexes/datetimes.py
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,8 @@ def date_range(
tz=None,
normalize: bool = False,
name: Hashable = None,
closed=None,
closed: str | None | lib.NoDefault = lib.no_default,
inclusive: str | None = None,
**kwargs,
) -> DatetimeIndex:
"""
Expand Down Expand Up @@ -919,6 +920,14 @@ def date_range(
closed : {None, 'left', 'right'}, optional
Make the interval closed with respect to the given frequency to
the 'left', 'right', or both sides (None, the default).

.. deprecated:: 1.4.0
Argument `closed` have been deprecated to standardize boundary inputs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'has' not 'have'.

Use `inclusive` instead, to set each bound as closed or open.
inclusive : {"both", "neither", "left", "right"}, default "both"
Include boundaries; Whether to set each bound as closed or open.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

versionadded 1.4.0


.. versionadded:: 1.4.0
**kwargs
For compatibility. Has no effect on the result.

Expand Down Expand Up @@ -1029,6 +1038,28 @@ def date_range(
DatetimeIndex(['2017-01-02', '2017-01-03', '2017-01-04'],
dtype='datetime64[ns]', freq='D')
"""
if inclusive is not None and not isinstance(closed, lib.NoDefault):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its possible we would want to share this (across functoions), but ok here

raise ValueError(
"Deprecated argument `closed` cannot be passed"
"if argument `inclusive` is not None"
)
elif not isinstance(closed, lib.NoDefault):
warnings.warn(
"Argument `closed` is deprecated in favor of `inclusive`.",
FutureWarning,
stacklevel=2,
)
if closed is None:
inclusive = "both"
elif closed in ("left", "right"):
inclusive = closed
else:
raise ValueError(
"Argument `closed` has to be either 'left', 'right' or None"
)
elif inclusive is None:
inclusive = "both"

if freq is None and com.any_none(periods, start, end):
freq = "D"

Expand All @@ -1039,7 +1070,7 @@ def date_range(
freq=freq,
tz=tz,
normalize=normalize,
closed=closed,
inclusive=inclusive,
**kwargs,
)
return DatetimeIndex._simple_new(dtarr, name=name)
Expand All @@ -1055,7 +1086,8 @@ def bdate_range(
name: Hashable = None,
weekmask=None,
holidays=None,
closed=None,
closed: lib.NoDefault = lib.no_default,
inclusive: str | None = None,
**kwargs,
) -> DatetimeIndex:
"""
Expand Down Expand Up @@ -1090,6 +1122,14 @@ def bdate_range(
closed : str, default None
Make the interval closed with respect to the given frequency to
the 'left', 'right', or both sides (None).

.. deprecated:: 1.4.0
Argument `closed` have been deprecated to standardize boundary inputs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'has'

Use `inclusive` instead, to set each bound as closed or open.
inclusive : {"both", "neither", "left", "right"}, default "both"
Include boundaries; Whether to set each bound as closed or open.

.. versionadded:: 1.4.0
**kwargs
For compatibility. Has no effect on the result.

Expand Down Expand Up @@ -1143,6 +1183,7 @@ def bdate_range(
normalize=normalize,
name=name,
closed=closed,
inclusive=inclusive,
**kwargs,
)

Expand Down
5 changes: 0 additions & 5 deletions pandas/tests/frame/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@
import pandas._testing as tm


@pytest.fixture(params=["both", "neither", "left", "right"])
def inclusive_endpoints_fixture(request):
return request.param


@pytest.fixture
def float_frame_with_na():
"""
Expand Down