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

REF: Replace args/kwargs in groupby.resample with TimeGrouper arguments #54961

Open
rhshadrach opened this issue Sep 2, 2023 · 4 comments
Open
Labels
Groupby Needs Discussion Requires discussion from core team before further action Refactor Internal refactoring of code Resample resample method

Comments

@rhshadrach
Copy link
Member

rhshadrach commented Sep 2, 2023

The args/kwargs to this method are currently documented as:

*args, **kwargs
Possible arguments are how, fill_method, limit, kind and on, and other arguments of TimeGrouper.

I think we should instead have these argument as being keyword arguments to groupby.resample and a test that ensures the arguments to TimeGrouper are a subset of the arguments to this method.

Also, it looks like some of these arguments, how, fill_method, and limit, are entirely ignored? But I haven't checked thoroughly.

cc @mroeschke

@rhshadrach rhshadrach added Refactor Internal refactoring of code Groupby Resample resample method Needs Discussion Requires discussion from core team before further action labels Sep 2, 2023
@mfenner1
Copy link

mfenner1 commented Dec 2, 2023

@rhshadrach Do you happen to know when TimeGrouper was resurrected? It was deprecated in 0.21.0: (see https://pandas.pydata.org/pandas-docs/stable/whatsnew/v0.21.0.html#deprecations and #16747).

But today, in main, TimeGrouper is a key member variable of class Resampler:

_timegrouper: TimeGrouper

In the deprecation comments (#16747 (comment)), it appears that TimeGrouper was a relic of the old-school groupby framework and its functionality was integrated into the Grouper interface.

So, I'm super curious if it was never fully removed, if it was only "deprecated" in terms of being an exposed/external capability, or if it was gone and has returned. Does anyone have any insight?

@rhshadrach
Copy link
Member Author

rhshadrach commented Dec 2, 2023

@mfenner1 - at one point in pandas there were two separate TimeGrouper classes:

class TimeGrouper(object):
def __new__(cls, *args, **kwargs):
from pandas.core.resample import TimeGrouper
import warnings
warnings.warn("pd.TimeGrouper is deprecated and will be removed; "
"Please use pd.Grouper(freq=...)",
FutureWarning, stacklevel=2)
return TimeGrouper(*args, **kwargs)

pandas/pandas/core/resample.py

Lines 1321 to 1323 in 51db82d

class TimeGrouper(Grouper):
"""
Custom groupby class for time-interval grouping.

The deprecation was only for pd.TimeGrouper. You can still get a TimeGrouper today by creating a Grouper with the freq argument not None. This was never deprecated, and shouldn't be.

@mfenner1
Copy link

mfenner1 commented Dec 3, 2023

@rhshadrach That is really useful! I didn't get that little detail. Thank you!

Can I follow up with another question or two?

Do both pd.Grouper(freq=) and pd.resample() lead to the same class TimeGrouper objects? Are there any explicit tests about guarantees between arguments and resulting TimeGroupers? [Maybe those questions play back into your original comments about handling the arguments .... Or maybe aligning those is exactly what you are proposing?]

Thanks,
Mark

@rhshadrach
Copy link
Member Author

Do both pd.Grouper(freq=) and pd.resample() lead to the same class TimeGrouper objects? Are there any explicit tests about guarantees between arguments and resulting TimeGroupers?

Offhand, I do not know. Would need to step through the code and search the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Groupby Needs Discussion Requires discussion from core team before further action Refactor Internal refactoring of code Resample resample method
Projects
None yet
Development

No branches or pull requests

2 participants