Skip to content
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: remove deprecated freqs/abbrevs 'A', 'A-DEC', 'A-JAN', etc. #57699

Conversation

natmokval
Copy link
Contributor

xref #55252

remove deprecated string 'A', 'A-DEC', etc. denoting timedelta abbreviations and timeseries frequencies

@@ -18,6 +18,7 @@ cdef dict c_DEPR_ABBREVS
cdef dict attrname_to_abbrevs
cdef dict npy_unit_to_attrname
cdef dict attrname_to_npy_unit
cdef set c_REMOVED_ABBREVS
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to keep track of these over time. Should just be able to remove from c_DEPR_ABBREVS and call it a day I think

@natmokval natmokval added the Deprecate Functionality to remove in pandas label Mar 6, 2024
@natmokval natmokval marked this pull request as ready for review March 6, 2024 22:57
@natmokval natmokval requested a review from MarcoGorelli as a code owner March 6, 2024 22:57
Comment on lines 285 to 299
@pytest.mark.parametrize(
"freq, freq_depr",
[
("1YE", "1A"),
("2YE-MAR", "2A-MAR"),
],
)
def test_asfreq_frequency_A_raisees(self, freq, freq_depr):
msg = f"Invalid frequency: {freq_depr[1:]}"

index = date_range("1/1/2000", periods=4, freq=f"{freq[1:]}")
df = DataFrame({"s": Series([0.0, 1.0, 2.0, 3.0], index=index)})

with pytest.raises(ValueError, match=msg):
df.asfreq(freq=freq_depr)
Copy link
Member

Choose a reason for hiding this comment

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

it's not clear to me what you're doing with freq in this test - I think index can just be instatiated with 'YE' in both cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think, in the first case we get df.index.freqstr = YE-DEC, and in second case df.index.freqstr =YE-MAR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, you are right, it's sufficient to check only freq="A". I simplified the test and added a note to v3.0.0.
Could you please take a look at my changes?

@natmokval natmokval requested a review from rhshadrach as a code owner March 7, 2024 13:17
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

looks good to me

good to explicitly check that these raise here, if there's something I've learned about all these offsets is that anything could happen 😄

@MarcoGorelli MarcoGorelli added this to the 3.0 milestone Mar 8, 2024
@natmokval
Copy link
Contributor Author

Thanks @MarcoGorelli for reviewing this PR.

@WillAyd WillAyd merged commit 7287487 into pandas-dev:main Mar 8, 2024
47 checks passed
@WillAyd
Copy link
Member

WillAyd commented Mar 8, 2024

Thanks @natmokval

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants