Skip to content

Refactor/validate utils #43551

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

Merged
merged 6 commits into from
Sep 14, 2021
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
33 changes: 0 additions & 33 deletions pandas/core/arrays/datetimelike.py
Original file line number Diff line number Diff line change
Expand Up @@ -1823,39 +1823,6 @@ def validate_periods(periods):
return periods


def validate_endpoints(closed):
"""
Check that the `closed` argument is among [None, "left", "right"]

Parameters
----------
closed : {None, "left", "right"}

Returns
-------
left_closed : bool
right_closed : bool

Raises
------
ValueError : if argument is not among valid values
"""
left_closed = False
right_closed = False

if closed is None:
left_closed = True
right_closed = True
elif closed == "left":
left_closed = True
elif closed == "right":
right_closed = True
else:
raise ValueError("Closed has to be either 'left', 'right' or None")

return left_closed, right_closed


def validate_inferred_freq(freq, inferred_freq, freq_infer):
"""
If the user passes a freq and another freq is inferred from passed data,
Expand Down
3 changes: 2 additions & 1 deletion pandas/core/arrays/datetimes.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
)
from pandas._typing import npt
from pandas.errors import PerformanceWarning
from pandas.util._validators import validate_endpoints

from pandas.core.dtypes.cast import astype_dt64_to_dt64tz
from pandas.core.dtypes.common import (
Expand Down Expand Up @@ -416,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 = dtl.validate_endpoints(closed)
left_closed, right_closed = validate_endpoints(closed)
start, end, _normalized = _maybe_normalize_endpoints(start, end, normalize)
tz = _infer_tz_from_endpoints(start, end, tz)

Expand Down
3 changes: 2 additions & 1 deletion pandas/core/arrays/timedeltas.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
NpDtype,
)
from pandas.compat.numpy import function as nv
from pandas.util._validators import validate_endpoints

from pandas.core.dtypes.cast import astype_td64_unit_conversion
from pandas.core.dtypes.common import (
Expand Down Expand Up @@ -312,7 +313,7 @@ def _generate_range(cls, start, end, periods, freq, closed=None):
if end is not None:
end = Timedelta(end)

left_closed, right_closed = dtl.validate_endpoints(closed)
left_closed, right_closed = validate_endpoints(closed)

if freq is not None:
index = generate_regular_range(start, end, periods, freq)
Expand Down
28 changes: 13 additions & 15 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
validate_ascending,
validate_bool_kwarg,
validate_fillna_kwargs,
validate_inclusive,
)

from pandas.core.dtypes.common import (
Expand Down Expand Up @@ -7694,16 +7695,18 @@ def between_time(
if not isinstance(index, DatetimeIndex):
raise TypeError("Index must be DatetimeIndex")

if (include_start != lib.no_default or include_end != lib.no_default) and (
inclusive is not None
):
old_include_arg_used = (include_start != lib.no_default) or (
include_end != lib.no_default
)

if old_include_arg_used and inclusive is not None:
raise ValueError(
"Deprecated arguments `include_start` and `include_end` "
"cannot be passed if `inclusive` has been given."
)
# If any of the deprecated arguments ('include_start', 'include_end')
# have been passed
elif (include_start != lib.no_default) or (include_end != lib.no_default):
elif old_include_arg_used:
warnings.warn(
"`include_start` and `include_end` are deprecated in "
"favour of `inclusive`.",
Expand All @@ -7720,20 +7723,15 @@ def between_time(
(False, False): "neither",
}
inclusive = inc_dict[(left, right)]
else: # On arg removal inclusive can default to "both"
if inclusive is None:
inclusive = "both"
elif inclusive not in ["both", "neither", "left", "right"]:
raise ValueError(
f"Inclusive has to be either string of 'both', "
f"'left', 'right', or 'neither'. Got {inclusive}."
)

elif inclusive is None:
# On arg removal inclusive can default to "both"
inclusive = "both"
left_inclusive, right_inclusive = validate_inclusive(inclusive)
indexer = index.indexer_between_time(
start_time,
end_time,
include_start=inclusive in ["both", "left"],
include_end=inclusive in ["both", "right"],
include_start=left_inclusive,
include_end=right_inclusive,
)
return self._take_with_is_copy(indexer, axis=axis)

Expand Down
5 changes: 1 addition & 4 deletions pandas/tests/frame/methods/test_between_time.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,7 @@ def test_between_time_incorr_arg_inclusive(self):
stime = time(0, 0)
etime = time(1, 0)
inclusive = "bad_string"
msg = (
"Inclusive has to be either string of 'both', 'left', 'right', "
"or 'neither'. Got bad_string."
)
msg = "Inclusive has to be either 'both', 'neither', 'left' or 'right'"
with pytest.raises(ValueError, match=msg):
ts.between_time(stime, etime, inclusive=inclusive)

Expand Down
40 changes: 40 additions & 0 deletions pandas/tests/util/test_validate_inclusive.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import numpy as np
import pytest

from pandas.util._validators import validate_inclusive

import pandas as pd


@pytest.mark.parametrize(
"invalid_inclusive",
(
"ccc",
2,
object(),
None,
np.nan,
pd.NA,
pd.DataFrame(),
),
)
def test_invalid_inclusive(invalid_inclusive):
with pytest.raises(
ValueError,
match="Inclusive has to be either 'both', 'neither', 'left' or 'right'",
):
validate_inclusive(invalid_inclusive)


@pytest.mark.parametrize(
"valid_inclusive, expected_tuple",
(
("left", (True, False)),
("right", (False, True)),
("both", (True, True)),
("neither", (False, False)),
),
)
def test_valid_inclusive(valid_inclusive, expected_tuple):
resultant_tuple = validate_inclusive(valid_inclusive)
assert expected_tuple == resultant_tuple
67 changes: 67 additions & 0 deletions pandas/util/_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,3 +427,70 @@ def validate_ascending(
return validate_bool_kwarg(ascending, "ascending", **kwargs)

return [validate_bool_kwarg(item, "ascending", **kwargs) for item in ascending]


def validate_endpoints(closed: str | None) -> tuple[bool, bool]:
"""
Check that the `closed` argument is among [None, "left", "right"]

Parameters
----------
closed : {None, "left", "right"}

Returns
-------
left_closed : bool
right_closed : bool

Raises
------
ValueError : if argument is not among valid values
"""
left_closed = False
right_closed = False

if closed is None:
left_closed = True
right_closed = True
elif closed == "left":
left_closed = True
elif closed == "right":
right_closed = True
else:
raise ValueError("Closed has to be either 'left', 'right' or None")

return left_closed, right_closed


def validate_inclusive(inclusive: str | None) -> tuple[bool, bool]:
"""
Check that the `inclusive` argument is among {"both", "neither", "left", "right"}.

Parameters
----------
inclusive : {"both", "neither", "left", "right"}

Returns
-------
left_right_inclusive : tuple[bool, bool]

Raises
------
ValueError : if argument is not among valid values
"""
left_right_inclusive: tuple[bool, bool] | None = None

if isinstance(inclusive, str):
left_right_inclusive = {
"both": (True, True),
"left": (True, False),
"right": (False, True),
"neither": (False, False),
}.get(inclusive)

if left_right_inclusive is None:
raise ValueError(
"Inclusive has to be either 'both', 'neither', 'left' or 'right'"
)

return left_right_inclusive