-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TYP: NDFrame.resample #30947
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
TYP: NDFrame.resample #30947
Conversation
@@ -5777,13 +5777,14 @@ def groupby( | |||
group_keys: bool = True, | |||
squeeze: bool = False, | |||
observed: bool = False, | |||
) -> "groupby_generic.DataFrameGroupBy": | |||
) -> "DataFrameGroupBy": | |||
from pandas.core.groupby.generic import DataFrameGroupBy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this no longer be done at the top of the module rather than in the function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its not a good idea to import the class at the top because of circular import problems.
There was a discussion in #30314 if the import could be less ugly (generic_groupby is not a module name). This is an attempt to improve on that and makes the import for groupbys and resample more similar to each other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. Generally not a fan of this but I guess somewhat orthogonal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@@ -5777,13 +5777,14 @@ def groupby( | |||
group_keys: bool = True, | |||
squeeze: bool = False, | |||
observed: bool = False, | |||
) -> "groupby_generic.DataFrameGroupBy": | |||
) -> "DataFrameGroupBy": | |||
from pandas.core.groupby.generic import DataFrameGroupBy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. Generally not a fan of this but I guess somewhat orthogonal
Thanks @topper-123 |
This PR types up the method
NDFrame.resample
.Also renames the func
resample
toget_resampler
and import the groupby types used for type checking in frame.py/series.py undir theif TYPE_CHECKING:
clause.