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

CLN: enforce deprecation of the kind keyword on df.resample/ser.resample #58125

Merged

Conversation

natmokval
Copy link
Contributor

@natmokval natmokval commented Apr 2, 2024

xref #55895
enforced deprecation of the kind keyword on df.resample/ser.resample

@natmokval natmokval added Clean Resample resample method labels Apr 8, 2024
@natmokval natmokval marked this pull request as ready for review April 8, 2024 16:56
@natmokval natmokval requested a review from jbrockmendel April 9, 2024 13:55
@natmokval
Copy link
Contributor Author

I enforced deprecation of the kind keyword on df.resample/ser.resample.
@jbrockmendel, could you please take a look at this PR?

@@ -449,9 +449,7 @@ def test_resample_frame_basic_kind(freq, unit):
index=date_range("2000-01-01", periods=10, freq="B"),
)
df.index = df.index.as_unit(unit)
msg = "The 'kind' keyword in DataFrame.resample is deprecated"
with tm.assert_produces_warning(FutureWarning, match=msg):
df.resample(freq, kind="period").mean()
Copy link
Member

Choose a reason for hiding this comment

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

i think you can remove this test entirely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, done

@@ -8976,7 +8955,6 @@ def resample(
freq=rule,
label=label,
closed=closed,
kind=kind,
Copy link
Member

Choose a reason for hiding this comment

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

i dont see a diff in pandas.core.resample; doesn't the keyword need to be removed there too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, my mistake. I removed the keyword kind from get_resampler

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't all/most of the kind keywords in resample.py be removable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment. Yes, it was my mistake; kind can be removed from Resampler and TimeGrouper. I have updated this PR. Could you please take a look?

@jbrockmendel
Copy link
Member

@ChadFulton any objection to moving forward with this part of the deprecation?

@ChadFulton
Copy link

Thanks @jbrockmendel, this change makes a lot of sense to me!

@@ -489,16 +479,17 @@ def test_cant_fill_missing_dups(self):
s.resample("Y").ffill()

@pytest.mark.parametrize("freq", ["5min"])
Copy link
Member

Choose a reason for hiding this comment

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

Could you inline this parameter since it's the only one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

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

One comment otherwise LGTM

@mroeschke mroeschke added this to the 3.0 milestone Apr 19, 2024
@mroeschke mroeschke merged commit 4a9a5ae into pandas-dev:main Apr 19, 2024
46 checks passed
@mroeschke
Copy link
Member

Thanks @natmokval

pmhatre1 pushed a commit to pmhatre1/pandas-pmhatre1 that referenced this pull request May 7, 2024
…ample (pandas-dev#58125)

* enforced deprecation of the kind keyword on df.resample/ser.resample

* fix tests

* fix tests

* fix tests

* add a note to v3.0.0

* return a blank line in resample.py back

* remove leftover

* remove kind from get_resampler, delete test

* remove kwd kind from _get_resampler, fix test

* remove kwd kind from the class Resampler

* remove kind from TimeGrouper

* inline the parameter freq in test_resample_5minute
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Resample resample method
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants