-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
DEPR offsets: rename 'Q' to 'QE' #55553
Changes from 11 commits
2105a16
752fc87
54ac99d
2c1b436
8c563a5
cb101a0
477071c
f1b7b29
0e89af9
a86b4c5
35db1b2
de976e1
2a3b30c
55c99b5
e563d6b
15cfdb1
29b199a
e71aaf3
261d996
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 |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
from pandas._libs import lib | ||
from pandas._libs.json import ujson_loads | ||
from pandas._libs.tslibs import timezones | ||
from pandas._libs.tslibs.dtypes import freq_to_period_freqstr | ||
from pandas.util._exceptions import find_stack_level | ||
|
||
from pandas.core.dtypes.base import _registry as registry | ||
|
@@ -207,6 +208,8 @@ def convert_json_field_to_pandas_type(field) -> str | CategoricalDtype: | |
if field.get("tz"): | ||
return f"datetime64[ns, {field['tz']}]" | ||
elif field.get("freq"): | ||
# GH#9586 rename frequency M to ME for offsets | ||
field["freq"] = freq_to_period_freqstr(1, field["freq"]) | ||
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. left over from #55581 (review)? should we wait for that one to be resolved first? 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. |
||
# GH#47747 using datetime over period to minimize the change surface | ||
return f"period[{field['freq']}]" | ||
else: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,7 +54,7 @@ def test_to_period_quarterly(self, month): | |
def test_to_period_quarterlyish(self, off): | ||
rng = date_range("01-Jan-2012", periods=8, freq=off) | ||
prng = rng.to_period() | ||
assert prng.freq == "Q-DEC" | ||
assert prng.freq == "QE-DEC" | ||
|
||
@pytest.mark.parametrize("off", ["BY", "YS", "BYS"]) | ||
def test_to_period_annualish(self, off): | ||
|
@@ -89,6 +89,23 @@ def test_dti_to_period_2monthish(self, freq_offset, freq_period): | |
|
||
tm.assert_index_equal(pi, period_range("2020-01", "2020-05", freq=freq_period)) | ||
|
||
@pytest.mark.parametrize( | ||
"freq, freq_depr", | ||
[ | ||
("2ME", "2M"), | ||
("2QE", "2Q"), | ||
("2QE-SEP", "2Q-SEP"), | ||
], | ||
) | ||
def test_to_period_freq_deprecated(self, freq, freq_depr): | ||
# GH#9586 | ||
msg = f"'{freq_depr[1:]}' will be deprecated, please use '{freq[1:]}' instead." | ||
|
||
rng = date_range("01-Jan-2012", periods=8, freq=freq) | ||
prng = rng.to_period() | ||
with tm.assert_produces_warning(UserWarning, match=msg): | ||
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. Looks like I'd missed this last time, but this should be FutureWarning, not UserWarning #52064 (review) could we address that in a separate PR please? 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. thank you, I corrected Warning class and made the separate PR #55636 |
||
assert prng.freq == freq_depr | ||
|
||
def test_to_period_infer(self): | ||
# https://github.com/pandas-dev/pandas/issues/33358 | ||
rng = date_range( | ||
|
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.
This looks very similar to the previous section, perhaps we can combine them?
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.
yes, I agree. I combined these two sections together.