Skip to content

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

Merged
merged 1 commit into from
Jan 13, 2020

Conversation

topper-123
Copy link
Contributor

@topper-123 topper-123 commented Jan 12, 2020

This PR types up the method NDFrame.resample.

Also renames the func resample to get_resampler and import the groupby types used for type checking in frame.py/series.py undir the if TYPE_CHECKING: clause.

@simonjayhawkins simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Jan 12, 2020
@simonjayhawkins simonjayhawkins added this to the 1.1 milestone Jan 12, 2020
@@ -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
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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

Copy link
Member

@WillAyd WillAyd left a 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
Copy link
Member

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

@simonjayhawkins simonjayhawkins merged commit 67fcdef into pandas-dev:master Jan 13, 2020
@simonjayhawkins
Copy link
Member

Thanks @topper-123

@topper-123 topper-123 deleted the type_ndframe_resample branch January 13, 2020 20:22
@topper-123 topper-123 mentioned this pull request Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants