Skip to content

TYP: series apply method adds type hints #39058

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 7 commits into from
Jan 11, 2021

Conversation

aniaan
Copy link
Contributor

@aniaan aniaan commented Jan 9, 2021

  • Ensure all linting tests pass, see here for how to run them

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @BEANNAN for the PR. a couple of suggestions.

def apply(self, func, convert_dtype=True, args=(), **kwds):
def apply(
self, func: AggFuncType, convert_dtype: bool = True, args: tuple = (), **kwds
):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
):
) -> FrameOrSeriesUnion:

@@ -3979,7 +3979,9 @@ def transform(
) -> FrameOrSeriesUnion:
return transform(self, func, axis, *args, **kwargs)

def apply(self, func, convert_dtype=True, args=(), **kwds):
def apply(
self, func: AggFuncType, convert_dtype: bool = True, args: tuple = (), **kwds
Copy link
Member

Choose a reason for hiding this comment

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

use Tuple from typing and add type parameters where possible

Suggested change
self, func: AggFuncType, convert_dtype: bool = True, args: tuple = (), **kwds
self, func: AggFuncType, convert_dtype: bool = True, args: Tuple[Any] = (), **kwds

@simonjayhawkins simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Jan 9, 2021
@simonjayhawkins simonjayhawkins added this to the 1.3 milestone Jan 9, 2021
@aniaan
Copy link
Contributor Author

aniaan commented Jan 9, 2021

Thanks @BEANNAN for the PR. a couple of suggestions.

Corrected, please take a look

@simonjayhawkins
Copy link
Member

@aniaan
Copy link
Contributor Author

aniaan commented Jan 9, 2021

CI has passed, please take a look

@jreback
Copy link
Contributor

jreback commented Jan 9, 2021

can you merge master; your changes will now work in pandas/core/apply.py where thing were moved.

@aniaan
Copy link
Contributor Author

aniaan commented Jan 10, 2021

Updated, but there was a slight problem with the CI that doesn't seem to be related to this commit

@simonjayhawkins
Copy link
Member

Updated, but there was a slight problem with the CI that doesn't seem to be related to this commit

yep, unrelated.

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @BEANNAN lgtm

FYI, the casts would be unnecessary with @overload, but OK with gradual typing to do things in small incremental steps.

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

lgtm

@simonjayhawkins simonjayhawkins merged commit 2ce81ae into pandas-dev:master Jan 11, 2021
luckyvs1 pushed a commit to luckyvs1/pandas that referenced this pull request Jan 20, 2021
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.

4 participants