type `freq` in `shift`, consistently use `Frequency` alias #1394

MarcoGorelli · 2025-09-30T13:07:12Z

use Frequency instead of str | BaseOffset / str | DateOffset
use timedelta instead of timedelta | Timedelta (as Timedelta inherits from timedelta anyway)

Dr-Irv · 2025-09-30T18:54:48Z

@MarcoGorelli can you resolve conflicts?

loicdiridollou

Can we just confirm that turning str | DateOffset to Frequency is good?
Wondering if there is an edge case for something that is a BaseOffset but not DateOffset.
Otherwise looks good to me, just fix the conflicts, will approve in advance, thanks @MarcoGorelli

MarcoGorelli · 2025-10-01T08:26:18Z

thanks for your reviews!

sorry for the lack of context, I should have linked to pandas-dev/pandas#60886 where there was some discussion on this (BaseOffset vs DateOffset)

loicdiridollou · 2025-10-01T11:20:54Z

pandas-stubs/core/indexes/accessors.pyi

    def to_timestamp(
        self,
-        freq: str | DateOffset | None = ...,
+        freq: Frequency | None = ...,


Okay so here is a good example:

idx = pd.PeriodIndex(["2023", "2024", "2025"], freq="D") s1 = pd.Series([1, 2, 3], index=idx) s1.to_timestamp(freq=pd.tseries.offsets.BDay())

This will raise a FutureWarning:

<stdin>:1: FutureWarning: PeriodDtype[B] is deprecated and will be removed in a future version. Use a DatetimeIndex with freq='B' instead

The issue is that BDay is not a DateOffset. I think it is a bit of an edge case and this situation works at runtime for now but good to think about the future, I am good with letting the types a bit looser here, @Dr-Irv what do you think?

loicdiridollou · 2025-10-01T11:23:48Z

@MarcoGorelli thanks for the context, my only little concern is that BDay is a BaseOffset and not DateOffset so that may give weird behaviors but since your PR makes the types a tiny bit looser in places where it was originally str | DateOffset I am good with the changes and we can investigate edge cases.

MarcoGorelli · 2025-10-01T13:30:32Z

thanks for your review

there's quite a few places where PeriodDtype[B] can't be used, so they should probably all use DateOffset | str

Dr-Irv · 2025-10-01T14:28:01Z

Here are my thoughts on this. The parameters need to align with the docs.

For example, I looked at timedelta_range . In this PR, it changes the freq parameter from str | DateOffset to Frequency, but that is too wide, and the docs specifically say DateOffset.

Might be worth having 2 separate types in _typing.pyi: One for Frequency that uses DateOffset and another that uses BaseOffset, but not sure about that.

@loicdiridollou Part of the review process here should be to compare each of the methods to the docs, and use that as a guide as to what goes in this PR.

MarcoGorelli · 2025-10-01T14:33:54Z

Conversely, there are some parts which currently are annotated with BaseOffset but should probably be DateOffset

In [39]: pd.Period('2020').asfreq(pd.offsets.BusinessDay())
<ipython-input-39-1953d30c79b5>:1: FutureWarning: Period with BDay freq is deprecated and will be removed in a future version. Use a DatetimeIndex with BDay freq instead.
  pd.Period('2020').asfreq(pd.offsets.BusinessDay())
Out[39]: Period('2020-12-31', 'B')

If BusinessDay is the only offset that this catches, then for what it's worth I don't think it's worth distinguishing them

MarcoGorelli · 2025-10-01T15:16:20Z

The issue is that BDay is not a DateOffset

Note that Day is also not assignable DateOffset, even though PeriodType[D] is perfectly valid (furthering my stance that they're not worth distinguishing them in stubs, but I'm happy to respect whatever decision is made here)

Dr-Irv · 2025-10-01T15:19:36Z

If BusinessDay is the only offset that this catches, then for what it's worth I don't think it's worth distinguishing them

I agree. I think there are other offsets that aren't supported as well, e.g., BusinessMonthEnd. We probably could go through offsets.pyx and figure out which classes have _period_dtype_code set and then create a type for that to get the proper restrictions. Not needed for this PR.

Dr-Irv · 2025-10-01T15:25:40Z

If BusinessDay is the only offset that this catches, then for what it's worth I don't think it's worth distinguishing them

I agree. I think there are other offsets that aren't supported as well, e.g., BusinessMonthEnd. We probably could go through offsets.pyx and figure out which classes have _period_dtype_code set and then create a type for that to get the proper restrictions. Not needed for this PR.

Actually via some grep, here's the list of valid offsets that can be used as a Period frequency:

Day | Hour | Minute | Second | Milli | Micro | Nano | YearEnd | QuarterEnd | MonthEnd | Week

@MarcoGorelli Can you create a type PeriodFrequency for that in _typing.pyi and use that instead?

MarcoGorelli · 2025-10-01T17:13:17Z

pandas-stubs/core/frame.pyi

    def to_timestamp(
        self,
-        freq=...,
+        freq: PeriodFrequency | None = None,


i've always found bizarre that to_timestamp only accepts period frequencies

In [4]: pd.Period('2020').to_timestamp('MS') --------------------------------------------------------------------------- AttributeError Traceback (most recent call last) Cell In[4], line 1 ----> 1 pd.Period('2020').to_timestamp('MS') File ~/scratch/.39venv/lib/python3.9/site-packages/pandas/_libs/tslibs/period.pyx:1868, in pandas._libs.tslibs.period._Period.to_timestamp() AttributeError: 'pandas._libs.tslibs.offsets.MonthBegin' object has no attribute '_period_dtype_code'

(and, on newer versions of pandas):

In [2]: pd.Period('2020').to_timestamp('MS') --------------------------------------------------------------------------- ValueError Traceback (most recent call last) File pandas/_libs/tslibs/offsets.pyx:5363, in pandas._libs.tslibs.offsets.to_offset() File pandas/_libs/tslibs/offsets.pyx:5246, in pandas._libs.tslibs.offsets._validate_to_offset_alias() ValueError: for Period, please use 'M' instead of 'MS' The above exception was the direct cause of the following exception: ValueError Traceback (most recent call last) Cell In[2], line 1 ----> 1 pd.Period('2020').to_timestamp('MS') File pandas/_libs/tslibs/period.pyx:2041, in pandas._libs.tslibs.period._Period.to_timestamp() File pandas/_libs/tslibs/period.pyx:1771, in pandas._libs.tslibs.period._Period._maybe_convert_freq() File pandas/_libs/tslibs/offsets.pyx:5402, in pandas._libs.tslibs.offsets.to_offset() ValueError: Invalid frequency: MS, failed to parse with error message: ValueError("for Period, please use 'M' instead of 'MS'")

but whatever, if that's how it is then it should at least be typed to respect that 🤷

MarcoGorelli · 2025-10-01T17:15:10Z

pandas-stubs/core/frame.pyi

-        freq: DateOffset | dt.timedelta | _str | None = ...,
+        freq: Frequency | dt.timedelta | None = ...,


I think the typing here was too tight to begin with

In [6]: s = pd.Series([1,2,3], index=pd.date_range('2020', freq='MS', periods=3)) In [7]: s.pct_change(freq='MS') Out[7]: 2020-01-01 NaN 2020-02-01 1.0 2020-03-01 0.5 Freq: MS, dtype: float64

MarcoGorelli · 2025-10-01T17:16:10Z

pandas-stubs/tseries/frequencies.pyi

 def to_offset(freq: None, is_period: bool = ...) -> None: ...
 @overload
-def to_offset(freq: Frequency, is_period: bool = ...) -> DateOffset: ...
+def to_offset(freq: Frequency, is_period: bool = ...) -> BaseOffset: ...


this is capable of returning frequencies beyond just the DateOffset ones

In [8]: pd.tseries.frequencies.to_offset('B') Out[8]: <BusinessDay>

MarcoGorelli · 2025-10-01T17:27:13Z

pandas-stubs/_libs/tslibs/timedeltas.pyi

-    def round(self, freq: str | BaseOffset) -> Self: ...
-    def floor(self, freq: str | BaseOffset) -> Self: ...
-    def ceil(self, freq: str | BaseOffset) -> Self: ...
+    def round(self, freq: Frequency) -> Self: ...
+    def floor(self, freq: Frequency) -> Self: ...
+    def ceil(self, freq: Frequency) -> Self: ...


docs say str but offset objects are also accepted, so I think it's ok to keep the current annotation

In [11]: pd.Timedelta(minutes=37).round(pd.tseries.offsets.Hour()) Out[11]: Timedelta('0 days 01:00:00')

type freq in shift, consistently use Frequency alias

8078382

MarcoGorelli marked this pull request as ready for review September 30, 2025 18:53

Dr-Irv requested a review from loicdiridollou September 30, 2025 18:54

loicdiridollou approved these changes Sep 30, 2025

View reviewed changes

Merge remote-tracking branch 'upstream/main' into unify-freq

8e2b00e

add some missing freq annotations#

83dd9c5

loicdiridollou reviewed Oct 1, 2025

View reviewed changes

MarcoGorelli added 3 commits October 1, 2025 14:12

restore DateOffset

1bfd52a

restore DateOffset

046e696

fixup

7b4194b

MarcoGorelli marked this pull request as draft October 1, 2025 13:29

MarcoGorelli added 2 commits October 1, 2025 16:05

separate more

6437417

Period.now

a47ead6

use PeriodFrequency

7af641f

MarcoGorelli commented Oct 1, 2025

View reviewed changes

MarcoGorelli marked this pull request as ready for review October 1, 2025 17:30

MarcoGorelli commented Oct 1, 2025

View reviewed changes

		freq: DateOffset \| dt.timedelta \| _str \| None = ...,
		freq: Frequency \| dt.timedelta \| None = ...,

Uh oh!

type freq in shift, consistently use Frequency alias #1394

Are you sure you want to change the base?

type freq in shift, consistently use Frequency alias #1394

Uh oh!

Conversation

MarcoGorelli commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dr-Irv commented Sep 30, 2025

Uh oh!

loicdiridollou left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarcoGorelli commented Oct 1, 2025

Uh oh!

loicdiridollou Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

loicdiridollou commented Oct 1, 2025

Uh oh!

MarcoGorelli commented Oct 1, 2025

Uh oh!

Dr-Irv commented Oct 1, 2025

Uh oh!

MarcoGorelli commented Oct 1, 2025

Uh oh!

MarcoGorelli commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dr-Irv commented Oct 1, 2025

Uh oh!

Dr-Irv commented Oct 1, 2025

Uh oh!

MarcoGorelli Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarcoGorelli Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

MarcoGorelli Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarcoGorelli Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

type `freq` in `shift`, consistently use `Frequency` alias #1394

type `freq` in `shift`, consistently use `Frequency` alias #1394

MarcoGorelli commented Sep 30, 2025 •

edited

Loading

loicdiridollou left a comment •

edited

Loading

MarcoGorelli commented Oct 1, 2025 •

edited

Loading

MarcoGorelli Oct 1, 2025 •

edited

Loading

MarcoGorelli Oct 1, 2025 •

edited

Loading