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

natmokval · 2023-12-05T22:58:16Z

xref #52064, #54061

Deprecated lowercase strings in favour of uppercase strings denoting:

period aliases weekly, monthly and bigger frequency
offsets aliases representing DateOffset subclasses that are a week or bigger (Week, MonthBegin, MonthEnd, etc.)
timedelta units for week, month or bigger

Deprecated uppercase strings in favour of lowercase strings denoting:

period aliases hourly, minutely or smaller frequency
offsets aliases representing DateOffset subclasses that are an hour or smaller (Hour, Minute, etc.)
timedelta units for hour, minute or smaller

…etc., fix tests

MarcoGorelli · 2023-12-07T12:00:02Z

pandas/_libs/tslibs/offsets.pyx

-                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":


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

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.

MarcoGorelli · 2023-12-07T12:55:08Z

pandas/_libs/tslibs/offsets.pyx

+                if is_period is False and name.upper() in c_OFFSET_DEPR_FREQSTR:
+                    if name == "m" and (n > 0 or stride == "-"):
+                        name = name
+                    else:


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(), )

?

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.

MarcoGorelli · 2023-12-14T09:58:00Z

pandas/_libs/tslibs/offsets.pyx

+        name not in _lite_rule_alias
+        and (name.upper() in _lite_rule_alias)
+        and name != "ms"


separate to this PR, but can we move 'ms', 'us', 'ns', and 'min' from _lite_rule_alias? _lit_rule_alias is only ever used with .get(name, name), so I don't think they need to be in it?

Thank you, I did as you suggested and removed "min", ”ms”, “ns", and “us" from _lite_rule_alias in separate PR #56516. I added these aliases to the list _dont_uppercase, because otherwise we uppercase them in _get_offset(). could you please take a look at this PR?

I am not sure, do we need “Min” in _lite_rule_alias? We want to deprecate the alias “Min” and left only lowercase “min” for Minutes. Am I correct?

MarcoGorelli · 2023-12-14T10:01:26Z

pandas/_libs/tslibs/offsets.pyx

+                    if n > 0 or stride == "-":
+                        name = name


this looks very complex, and not sure it's necessary anyway?

I agree, but I think it's the only way to pass test_to_offset_invalid() in pandas/tests/tslibs/test_to_offset.py
The reason is: when we parse frequency like "2h20m" or "-m" we show our warning FutureWarning: 'm' is deprecated and will be removed in a future version, please use 'ME' instead.
But here 'm' is offsets.Minute(), not offsets.MonthEnd()

sure but '2h20m' is already invalid to begin with?

I'm not sure we should worry about a false positive warning being raised for frequencies which were invalid to begin with

thanks, understood. I removed from to_offset() this unnecessary check and corrected parameters in test_to_offset_invalid to avoid raising the false positive warning

MarcoGorelli · 2023-12-19T17:54:57Z

pandas/tests/scalar/period/test_period.py

-        i2 = Period("1982", freq="MIN")
+        i2 = Period("1982-01-01 00:00")


why change this one completely? I'd suggest just to assert that the warning is raised and still check they're equal

why change this one completely?

I wanted to do assert and I thought this is proper way to define i2.

I'd suggest just to assert that the warning is raised and still check they're equal

thanks, it's a good idea. I did as you suggested.

….rst

MarcoGorelli

minor comment, but I think this looks good!

MarcoGorelli · 2023-12-21T14:02:10Z

doc/source/whatsnew/v2.2.0.rst

+Deprecate lowercase strings ``w``, ``m``, ``q``, etc. and uppercase strings ``H``, ``MIN``, ``S``, etc. for time series, period, and timedelta
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Previously, both ``'h'`` and ``'H'`` would allowed for 'hour' offset/period alias. We now require the case to be correct - check the :ref:`offset aliases <timeseries.offset_aliases>` and the :ref:`period aliases <timeseries.period_aliases>` parts of the docs and make sure you're using the correct one (:issue:`56346`)


"would allowed" -> "were allowed"

thank you, I corrected this.

MarcoGorelli · 2023-12-22T13:54:36Z

pandas/_libs/tslibs/offsets.pyx

+                elif (is_period is False and
+                        name != name.upper() and
+                        name.upper() in c_REVERSE_OFFSET_DEPR_FREQSTR):
+                    warnings.warn(
+                        f"\'{name}\' is deprecated and will be removed "
+                        f"in a future version, please use "
+                        f"\'{name.upper()}\' instead.",
+                        FutureWarning,
+                        stacklevel=find_stack_level(),
+                    )
+                    name = name.upper()


Is the idea here that if someone passes 'me' instead of 'ME', then it will warn?

I'm not sure this is the right place for this warning to be honest - like this it'll only show the warning for lowercase versions of aliases which have been renamed. For example:

In [5]: to_offset('qs') Out[5]: <QuarterBegin: startingMonth=1>

doesn't emit any warning, and similarly for others which aren't part of that list

thank you for noticing this, it's my mistake. I fixed it and updated the PR.

MarcoGorelli · 2023-12-22T14:06:56Z

pandas/_libs/tslibs/offsets.pyx

@@ -4795,40 +4794,60 @@ 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:
+                if is_period is False and name.upper() in c_OFFSET_DEPR_FREQSTR:


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

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).

Update time offsets to reflect changes in more recent versions of pandas (pandas-dev/pandas#56346).

github-actions · 2024-03-10T00:06:12Z

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

MarcoGorelli · 2024-03-13T11:15:23Z

shall we mark this as draft until we've enforced the deprecations already in?

natmokval · 2024-03-13T11:41:09Z

shall we mark this as draft until we've enforced the deprecations already in?

thank you, good idea. I marked this PR as draft.

natmokval · 2024-06-27T13:45:57Z

partially closed via #59051 and #58998.
It's still allowed to use mixed cases such as: "Min", "Us", "uS", "Ns", etc.

natmokval added 3 commits December 5, 2023 23:55

deprecate lowercase strings denoting freq for week, month, monthend, …

c7d82ec

…etc., fix tests

fix tests

614710b

fix tests

458a9c1

MarcoGorelli reviewed Dec 7, 2023

View reviewed changes

correct def _get_offset, fix tests

e9c8af5

natmokval mentioned this pull request Dec 8, 2023

ENH: unify warning message in to_offset() #56417

Merged

natmokval added 5 commits December 11, 2023 00:46

add tests, fix tests

73f68f0

fix tests

eea9b69

correct parse_timedelta_unit, to_offset, fix tests, add tests

386eb57

resolve conflicts, fix tests, add tests

5dcb918

fix tests

25bfbef

MarcoGorelli reviewed Dec 14, 2023

View reviewed changes

resolve conflicts, depr 'MIN' from to_timedelta, fix tests

eb418eb

natmokval mentioned this pull request Dec 15, 2023

CLN: remove unused entries from _lite_rule_alias #56516

Closed

natmokval added 3 commits December 15, 2023 14:20

Merge branch 'main' into depr-uppercasing-in-get-offset

943b898

deprecate 'Min' in favour of 'min'

435db76

correct docs

3cc94d2

MarcoGorelli reviewed Dec 19, 2023

View reviewed changes

natmokval added 7 commits December 19, 2023 23:42

show depr warning in test_construction() for Period

457ae96

resolve conflict

3837b7f

correct warning message in test_construction()

f709557

remove from to_offset() unnecessary check, fix test_to_offset_invalid

7cd909f

fix pre-commit error

8d1d6dd

Merge branch 'main' into depr-uppercasing-in-get-offset

6c24541

add notes to /v2.2.0.rst

286b618

natmokval marked this pull request as ready for review December 21, 2023 09:59

natmokval added 2 commits December 21, 2023 13:35

add filterwarnings to test_to_offset_invalid, correct notes in v2.2.0…

606d0c5

….rst

improve the headline in v2.2.0.rst

32993a9

natmokval changed the title ~~DEPR: lowercase strings denoting freq for Week, MonthBegin, MonthEnd, etc.~~ DEPR: lower/uppercase strings such as 'y', 'q', 'H', 'MIN', etc. denoting freqs/units for time series, period, and timedelta Dec 21, 2023

MarcoGorelli reviewed Dec 21, 2023

View reviewed changes

natmokval added 2 commits December 21, 2023 18:57

correct depr note in v2.2.0.rst

dcf1994

Merge branch 'main' into depr-uppercasing-in-get-offset

5443b1e

MarcoGorelli mentioned this pull request Dec 21, 2023

RLS: 2.2.0 #54902

Closed

MarcoGorelli suggested changes Dec 22, 2023

View reviewed changes

MarcoGorelli reviewed Dec 22, 2023

View reviewed changes

natmokval added 3 commits December 27, 2023 23:48

correct to_offset() for freqs such us ys, qs, and add tests

7889c7d

Merge branch 'main' into depr-uppercasing-in-get-offset

00e80c0

resolve conflicts, fix tests

0eb098e

natmokval added Deprecate Functionality to remove in pandas Frequency DateOffsets labels Jan 16, 2024

natmokval mentioned this pull request Jan 16, 2024

DEPR: lowercase freqs 'ye', 'qe', etc. raise a ValueError #56910

Merged

2 tasks

natmokval mentioned this pull request Jan 30, 2024

CLN: Move capitalize_first_letter to where it's used #57096

Merged

mps01060 added a commit to mps01060/intense-qc that referenced this pull request Feb 5, 2024

Update time offset for more recent pandas

ffa4d38

Update time offsets to reflect changes in more recent versions of pandas (pandas-dev/pandas#56346).

natmokval added 3 commits February 8, 2024 01:08

resolve conflicts

b6074b1

deprecate lowercase freq 'w', 'd' from timeseries

530f056

fix tests for 'D'

f600538

github-actions bot added the Stale label Mar 10, 2024

MarcoGorelli removed the Stale label Mar 13, 2024

natmokval marked this pull request as draft March 13, 2024 11:39

natmokval closed this Jun 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

natmokval commented Dec 5, 2023 •

edited

Loading

MarcoGorelli Dec 7, 2023

natmokval Dec 7, 2023

MarcoGorelli Dec 7, 2023

natmokval Dec 14, 2023

MarcoGorelli Dec 14, 2023

natmokval Dec 15, 2023

MarcoGorelli Dec 14, 2023

natmokval Dec 14, 2023 •

edited

Loading

MarcoGorelli Dec 19, 2023

natmokval Dec 20, 2023 •

edited

Loading

MarcoGorelli Dec 19, 2023

natmokval Dec 19, 2023

MarcoGorelli left a comment

MarcoGorelli Dec 21, 2023

natmokval Dec 21, 2023

MarcoGorelli Dec 22, 2023

natmokval Dec 28, 2023

MarcoGorelli Dec 22, 2023

natmokval Dec 28, 2023

github-actions bot commented Mar 10, 2024

MarcoGorelli commented Mar 13, 2024

natmokval commented Mar 13, 2024

natmokval commented Jun 27, 2024

		i2 = Period("1982", freq="MIN")
		i2 = Period("1982-01-01 00:00")

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

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

Conversation

natmokval commented Dec 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natmokval Dec 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natmokval Dec 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarcoGorelli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 10, 2024

MarcoGorelli commented Mar 13, 2024

natmokval commented Mar 13, 2024

natmokval commented Jun 27, 2024

natmokval commented Dec 5, 2023 •

edited

Loading

natmokval Dec 14, 2023 •

edited

Loading

natmokval Dec 20, 2023 •

edited

Loading