DEPR: remove deprecated freqs/abbrevs 'A', 'A-DEC', 'A-JAN', etc. by natmokval · Pull Request #57699 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation7 Commits6 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
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 |
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
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) |
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?
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
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?
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 😄
pmhatre1 pushed a commit to pmhatre1/pandas-pmhatre1 that referenced this pull request
