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 offsets: rename 'Q' to 'QE' #55553

Merged
merged 19 commits into from
Oct 24, 2023

Conversation

natmokval
Copy link
Contributor

@natmokval natmokval commented Oct 16, 2023

xref #9586, #52064

Deprecated the alias denoting quarter end frequency "Q" for offsets in favour of "QE".

@natmokval natmokval marked this pull request as ready for review October 19, 2023 21:30
@natmokval natmokval added Deprecate Functionality to remove in pandas Frequency DateOffsets labels Oct 20, 2023
@natmokval
Copy link
Contributor Author

@MarcoGorelli, could you please take a look at this PR? It's a follow-up of PR: "DEPR offsets: rename 'M' to 'ME'".
seems like ci failure isn't related to my changes


rng = date_range("01-Jan-2012", periods=8, freq=freq)
prng = rng.to_period()
with tm.assert_produces_warning(UserWarning, match=msg):
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

@natmokval natmokval Oct 22, 2023

Choose a reason for hiding this comment

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

thank you, I corrected Warning class and made the separate PR #55636

Comment on lines 164 to 165
def test_my(self):
Period("2017Q1", freq="Q")
Copy link
Member

Choose a reason for hiding this comment

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

temporary test name?😉

pandas/tests/scalar/period/test_period.py Show resolved Hide resolved
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, just this part isn't totally clear to me https://github.com/pandas-dev/pandas/pull/55553/files#r1367138011

Comment on lines 235 to 236
Deprecate alias ``Q`` in favour of ``QE`` for offsets
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Copy link
Member

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?

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, I agree. I combined these two sections together.

Comment on lines 211 to 212
# GH#9586 rename frequency M to ME for offsets
field["freq"] = freq_to_period_freqstr(1, field["freq"])
Copy link
Member

Choose a reason for hiding this comment

The 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?

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, it' related to #55581. I agree, it's better at first to merge #55581. I rolled my changes in pandas/io/json/_table_schema.py back , but now I have failures in test_read_json_table_period_orient, which will be resolved after merging.

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.

cool, should be good on green!

pandas/tests/scalar/period/test_period.py Show resolved Hide resolved
@MarcoGorelli
Copy link
Member

looks like there's some test failures

 FAILED pandas/tests/io/json/test_json_table_schema.py::TestTableOrientReader::test_read_json_table_period_orient[vals0-None] - TypeError: data type 'period[QE-DEC]' not understood

@natmokval
Copy link
Contributor Author

looks like there's some test failures

 FAILED pandas/tests/io/json/test_json_table_schema.py::TestTableOrientReader::test_read_json_table_period_orient[vals0-None] - TypeError: data type 'period[QE-DEC]' not understood

test_read_json_table_period_orient failed because there was a bug for conversion JSON field descriptor into pandas type for deprecated offsets frequency 'M'. We fixed it in #55581, now the test passes.

I added parameters 'Q' and '2Q' into parametrization of test_read_json_table_orient_period_depr_freq

@MarcoGorelli
Copy link
Member

thanks @natmokval !

@MarcoGorelli MarcoGorelli merged commit e0a8443 into pandas-dev:main Oct 24, 2023
33 checks passed
tacaswell added a commit to tacaswell/pandas that referenced this pull request May 29, 2024
The PRs pandas-dev#55792 (Y), pandas-dev#52064 (Q), and pandas-dev#55553 (M) deprecated the single letter
version of the aliases in favour of the -end version of them.

This table was missed as part of that work.
tacaswell added a commit to tacaswell/pandas that referenced this pull request May 29, 2024
The PRs pandas-dev#55792 (Y), pandas-dev#52064 (Q), and pandas-dev#55553 (M) deprecated the single letter
version of the aliases in favour of the -end version of them.

Add a note to the offset table about deprecations.
MarcoGorelli pushed a commit that referenced this pull request May 31, 2024
DOC: Add note about alias deprecations

The PRs #55792 (Y), #52064 (Q), and #55553 (M) deprecated the single letter
version of the aliases in favour of the -end version of them.

Add a note to the offset table about deprecations.
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 Frequency DateOffsets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants