Skip to content

DEPR: lower/uppercase strings such as 'y', 'q', 'H', 'MIN', etc. denoting freqs/units for time series, period, and timedelta #56346

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.

Sign up for GitHub

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

Closed
wants to merge 30 commits into from
Closed

DEPR: lower/uppercase strings such as 'y', 'q', 'H', 'MIN', etc. denoting freqs/units for time series, period, and timedelta #56346

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c7d82ec
deprecate lowercase strings denoting freq for week, month, monthend, …
natmokval Dec 5, 2023
614710b
fix tests
natmokval Dec 5, 2023
458a9c1
fix tests
natmokval Dec 6, 2023
e9c8af5
correct def _get_offset, fix tests
natmokval Dec 7, 2023
73f68f0
add tests, fix tests
natmokval Dec 7, 2023
eea9b69
fix tests
natmokval Dec 11, 2023
386eb57
correct parse_timedelta_unit, to_offset, fix tests, add tests
natmokval Dec 11, 2023
5dcb918
resolve conflicts, fix tests, add tests
natmokval Dec 12, 2023
25bfbef
fix tests
natmokval Dec 12, 2023
eb418eb
resolve conflicts, depr 'MIN' from to_timedelta, fix tests
natmokval Dec 14, 2023
943b898
Merge branch 'main' into depr-uppercasing-in-get-offset
natmokval Dec 15, 2023
435db76
deprecate 'Min' in favour of 'min'
natmokval Dec 15, 2023
3cc94d2
correct docs
natmokval Dec 15, 2023
457ae96
show depr warning in test_construction() for Period
natmokval Dec 19, 2023
3837b7f
resolve conflict
natmokval Dec 19, 2023
f709557
correct warning message in test_construction()
natmokval Dec 20, 2023
7cd909f
remove from to_offset() unnecessary check, fix test_to_offset_invalid
natmokval Dec 20, 2023
8d1d6dd
fix pre-commit error
natmokval Dec 20, 2023
6c24541
Merge branch 'main' into depr-uppercasing-in-get-offset
natmokval Dec 20, 2023
286b618
add notes to /v2.2.0.rst
natmokval Dec 21, 2023
606d0c5
add filterwarnings to test_to_offset_invalid, correct notes in v2.2.0…
natmokval Dec 21, 2023
32993a9
improve the headline in v2.2.0.rst
natmokval Dec 21, 2023
dcf1994
correct depr note in v2.2.0.rst
natmokval Dec 21, 2023
5443b1e
Merge branch 'main' into depr-uppercasing-in-get-offset
natmokval Dec 21, 2023
7889c7d
correct to_offset() for freqs such us ys, qs, and add tests
natmokval Dec 27, 2023
00e80c0
Merge branch 'main' into depr-uppercasing-in-get-offset
natmokval Dec 27, 2023
0eb098e
resolve conflicts, fix tests
natmokval Jan 4, 2024
b6074b1
resolve conflicts
natmokval Feb 8, 2024
530f056
deprecate lowercase freq 'w', 'd' from timeseries
natmokval Feb 8, 2024
f600538
fix tests for 'D'
natmokval Feb 8, 2024
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
89 changes: 45 additions & 44 deletions pandas/_libs/tslibs/offsets.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4554,29 +4554,7 @@ _lite_rule_alias = {
"ns": "ns",
}

_dont_uppercase = {
"h",
"bh",
"cbh",
"MS",
"ms",
"s",
"me",
"qe",
"qe-dec",
"qe-jan",
"qe-feb",
"qe-mar",
"qe-apr",
"qe-may",
"qe-jun",
"qe-jul",
"qe-aug",
"qe-sep",
"qe-oct",
"qe-nov",
"ye",
}
_dont_uppercase = {"h", "bh", "cbh", "MS", "ms", "s"}


INVALID_FREQ_ERR_MSG = "Invalid frequency: {0}"
Expand All @@ -4595,7 +4573,8 @@ def _get_offset(name: str) -> BaseOffset:
--------
_get_offset('EOM') --> BMonthEnd(1)
"""
if name.lower() not in _dont_uppercase:
name_orig = name
if name not in _dont_uppercase:
name = name.upper()
name = _lite_rule_alias.get(name, name)
name = _lite_rule_alias.get(name.lower(), name)
Expand All @@ -4612,7 +4591,14 @@ def _get_offset(name: str) -> BaseOffset:
except (ValueError, TypeError, KeyError) as err:
# bad prefix or suffix
raise ValueError(INVALID_FREQ_ERR_MSG.format(
f"{name}, failed to parse with error message: {repr(err)}")
f"{name_orig}, failed to parse with error message: {repr(err)}")
)
if name != name_orig and name_orig != "d":
Copy link
Member

Choose a reason for hiding this comment

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

sorry could you remind me why we're special-casing 'd' please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, my mistake. I did this because I got failures in tests for freq="d", e.g.

pandas/tests/resample/test_base.py -k test_resample_empty_dtypes
pandas/tests/tslibs/test_to_offset.py -k test_to_offset_leading_plus

but I had to replace deprecated lowercase frequency 'd' with the uppercase 'D'. Am I correct, we want to use for Day uppercase 'D'?
I will remove the check name_orig != "d" and will correct tests.

warnings.warn(
f"\'{name_orig}\' is deprecated and will be removed in a future "
f"version, please use \'{name_orig.upper()}\' instead.",
FutureWarning,
stacklevel=find_stack_level(),
)
# cache
_offset_map[name] = offset
Expand Down Expand Up @@ -4688,39 +4674,54 @@ cpdef to_offset(freq, bint is_period=False):

tups = zip(split[0::4], split[1::4], split[2::4])
for n, (sep, stride, name) in enumerate(tups):
if is_period is False and name in c_OFFSET_DEPR_FREQSTR:
warnings.warn(
f"\'{name}\' is deprecated, please use "
f"\'{c_OFFSET_DEPR_FREQSTR.get(name)}\' instead.",
FutureWarning,
stacklevel=find_stack_level(),
)
name = c_OFFSET_DEPR_FREQSTR[name]
if is_period is True and name in c_REVERSE_OFFSET_DEPR_FREQSTR:
if name.startswith("Y"):
if is_period is False and name.upper() in c_OFFSET_DEPR_FREQSTR:
Copy link
Member

Choose a reason for hiding this comment

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

also, little nitpick, but if not is_period is generally preferred over if is_period is False (and similarly for the True counterparts)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, if not is_period looks much better indeed. I replaced if is_period is False with it (and if is_period is True with if is_period).

if name == "m" and (n > 0 or stride == "-"):
name = name
else:
Copy link
Member

Choose a reason for hiding this comment

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

this is super-complicated, I can't understand it anymore

would it work to just keep this part as it is, and at the beginning of _get_offset, to add

    if name not in _lite_rule_alias and (name.upper() in _lite_rule_alias):
        warnings.warn(
            f"\'{name}\' is deprecated and will be removed in a future "
            f"version, please use \'{name.upper()}\' instead.",
            FutureWarning,
            stacklevel=find_stack_level(),
        )
    elif name not in _lite_rule_alias and (name.lower() in _lite_rule_alias):
        warnings.warn(
            f"\'{name}\' is deprecated and will be removed in a future "
            f"version, please use \'{name.lower()}\' instead.",
            FutureWarning,
            stacklevel=find_stack_level(),
        )

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the comment, I added it in _get_offset() and we do need this in some cases. But it only works for frequencies in _lite_rule_alias.
The problem I tried to resolve by doing the check

if name == "m" and (n > 0 or stride == "-"):

was for frequencies like "2h30m". When we pass this freq to to_offset() we show our warning: 'm' is deprecated and will be removed in a future version, please use 'ME' instead., but we shouldn't.

warnings.warn(
f"\'{name}\' is deprecated, please use "
f"\'{c_OFFSET_DEPR_FREQSTR.get(name.upper())}\' instead.",
FutureWarning,
stacklevel=find_stack_level(),
)
name = c_OFFSET_DEPR_FREQSTR[name.upper()]
if is_period is True and name.upper() in c_REVERSE_OFFSET_DEPR_FREQSTR:
if name.upper().startswith("Y"):
raise ValueError(
f"for Period, please use \'Y{name[2:]}\' "
f"for Period, please use \'Y{name.upper()[2:]}\' "
f"instead of \'{name}\'"
)
if (name.startswith("B") or
name.startswith("S") or name.startswith("C")):
if (name.upper().startswith("B") or
name.upper().startswith("S") or
name.upper().startswith("C")):
raise ValueError(INVALID_FREQ_ERR_MSG.format(name))
else:
raise ValueError(
f"for Period, please use "
f"\'{c_REVERSE_OFFSET_DEPR_FREQSTR.get(name)}\' "
f"\'{c_REVERSE_OFFSET_DEPR_FREQSTR.get(name.upper())}\' "
f"instead of \'{name}\'"
)
elif is_period is True and name in c_OFFSET_DEPR_FREQSTR:
if name.startswith("A"):
elif is_period is True and name.upper() in c_OFFSET_DEPR_FREQSTR:
if name.upper().startswith("A"):
warnings.warn(
f"\'{name}\' is deprecated and will be removed in a future "
f"version, please use \'{c_DEPR_ABBREVS.get(name)}\' "
f"instead.",
f"version, please use "
f"\'{c_DEPR_ABBREVS.get(name.upper())}\' instead.",
FutureWarning,
stacklevel=find_stack_level(),
)
name = c_OFFSET_DEPR_FREQSTR.get(name)
if name == "m" and (n > 0 or stride == "-"):
name = name
else:
if name.upper() != name:
warnings.warn(
f"\'{name}\' is deprecated and will be removed in "
f"a future version, please use \'{name.upper()}\' "
f"instead.",
FutureWarning,
stacklevel=find_stack_level(),
)
name = c_OFFSET_DEPR_FREQSTR.get(name.upper())

if sep != "" and not sep.isspace():
raise ValueError("separator must be spaces")
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/indexes/datetimes/test_partial_slicing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ def test_partial_slice_second_precision(self):
rng = date_range(
start=datetime(2005, 1, 1, 0, 0, 59, microsecond=999990),
periods=20,
freq="US",
freq="us",
)
s = Series(np.arange(20), rng)

Expand Down
4 changes: 2 additions & 2 deletions pandas/tests/resample/test_period_index.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,12 @@ def test_basic_downsample(self, simple_period_range_series):
("Y-DEC", "<YearEnd: month=12>"),
("Q-MAR", "<QuarterEnd: startingMonth=3>"),
("M", "<MonthEnd>"),
("w-thu", "<Week: weekday=3>"),
("W-THU", "<Week: weekday=3>"),
],
)
def test_not_subperiod(self, simple_period_range_series, rule, expected_error_msg):
# These are incompatible period rules for resampling
ts = simple_period_range_series("1/1/1990", "6/30/1995", freq="w-wed")
ts = simple_period_range_series("1/1/1990", "6/30/1995", freq="W-WED")
msg = (
"Frequency <Week: weekday=2> cannot be resampled to "
f"{expected_error_msg}, as they are not sub or super periods"
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/scalar/period/test_period.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_construction(self):
assert i1 == i3

i1 = Period("1982", freq="min")
i2 = Period("1982", freq="MIN")
i2 = Period("1982", freq="Min")
assert i1 == i2

i1 = Period(year=2005, month=3, day=1, freq="D")
Expand Down
4 changes: 2 additions & 2 deletions pandas/tests/tseries/offsets/test_offsets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,8 +784,8 @@ def test_get_offset():
pairs = [
("B", BDay()),
("b", BDay()),
("bme", BMonthEnd()),
("Bme", BMonthEnd()),
("BME", BMonthEnd()),
("BME", BMonthEnd()),
("W-MON", Week(weekday=0)),
("W-TUE", Week(weekday=1)),
("W-WED", Week(weekday=2)),
Expand Down