-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
CLN: replace unnecessary freq_to_period_freqstr
with PeriodDtype
constructor
#56550
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
Changes from 15 commits
bc53d16
2a3d992
42fa6c4
4905c42
afcffbb
6f9d974
5a4bdc5
e2c94f6
602a71b
b529965
daf707b
1b81967
b74807d
751ab49
48292cd
ec25560
a057a63
2131579
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,7 +34,6 @@ | |
Timestamp, | ||
to_offset, | ||
) | ||
from pandas._libs.tslibs.dtypes import freq_to_period_freqstr | ||
from pandas._typing import ( | ||
AlignJoin, | ||
AnyArrayLike, | ||
|
@@ -123,6 +122,7 @@ | |
from pandas.core.dtypes.dtypes import ( | ||
DatetimeTZDtype, | ||
ExtensionDtype, | ||
PeriodDtype, | ||
) | ||
from pandas.core.dtypes.generic import ( | ||
ABCDataFrame, | ||
|
@@ -10228,9 +10228,9 @@ def _shift_with_freq(self, periods: int, axis: int, freq) -> Self: | |
if freq != orig_freq: | ||
assert orig_freq is not None # for mypy | ||
raise ValueError( | ||
f"Given freq {freq_to_period_freqstr(freq.n, freq.name)} " | ||
f"Given freq {PeriodDtype(freq)._freqstr} " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this change the message? if so, could you show before vs after please? if not, then is it possible to do this replacement across the whole codebase and get rid of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It doesn't change the message.
I replaced |
||
f"does not match PeriodIndex freq " | ||
f"{freq_to_period_freqstr(orig_freq.n, orig_freq.name)}" | ||
f"{PeriodDtype(orig_freq)._freqstr}" | ||
) | ||
new_ax: Index = index.shift(periods) | ||
else: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
timedelta, | ||
) | ||
import pickle | ||
import re | ||
|
||
import numpy as np | ||
import pytest | ||
|
@@ -14,7 +15,8 @@ | |
BaseOffset, | ||
to_offset, | ||
) | ||
from pandas._libs.tslibs.dtypes import freq_to_period_freqstr | ||
|
||
from pandas.core.dtypes.dtypes import PeriodDtype | ||
|
||
from pandas import ( | ||
DataFrame, | ||
|
@@ -238,7 +240,8 @@ def test_line_plot_period_mlt_frame(self, frqncy): | |
index=idx, | ||
columns=["A", "B", "C"], | ||
) | ||
freq = freq_to_period_freqstr(1, df.index.freq.rule_code) | ||
freq = re.split(r"\d", PeriodDtype(df.index.freq)._freqstr)[-1] | ||
|
||
freq = df.index.asfreq(freq).freq | ||
_check_plot_works(df.plot, freq) | ||
|
||
|
@@ -253,7 +256,7 @@ def test_line_plot_datetime_frame(self, freq): | |
index=idx, | ||
columns=["A", "B", "C"], | ||
) | ||
freq = freq_to_period_freqstr(1, df.index.freq.rule_code) | ||
freq = re.split(r"\d", PeriodDtype(df.index.freq)._freqstr)[-1] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why does this one require regex splitting, but others don't? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, seems like we don't need this regex splitting here and can replace I will make this change. There is another test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think in
to
and then get rid of regex splitting. I modified the test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we use the same replacement in
with
the test passes. Should we modify this test as well? |
||
freq = df.index.to_period(freq).freq | ||
_check_plot_works(df.plot, freq) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need the if-else branch for
other
but not forself.freq
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.freq
always has attribute"_period_dtype_code",
burother
sometimes doesn't.For example in many tests in
pandas/tests/arithmetic/test_period.py
( e.g.test_pi_add_sub_timedeltalike_freq_mismatch_monthly
) other is equalpd.offsets.YearBegin(2)