Skip to content

Fix annotations for DataFrame.apply() including additional overloads #448

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 5 commits into from
Dec 5, 2022

Conversation

gandhis1
Copy link
Contributor

  • Tests added: Please use assert_type() to assert the type of any return value

In #401 the annotations for apply was changed. This introduced false negatives in my code base, because the particular mix of return types and arguments I was using was not supported. Here I am still not sure it is exhaustive but I think it's an improvement. In particular the result_type needs to be accounted for, as well as differentiating between a true list-like, str, and Series in the return value of the function.

@Dr-Irv @bashtage @danielroseman

@@ -155,6 +156,9 @@ AxisType: TypeAlias = Literal["columns", "index", 0, 1]
DtypeNp = TypeVar("DtypeNp", bound=np.dtype[np.generic])
KeysArgType: TypeAlias = Any
ListLike = TypeVar("ListLike", Sequence, np.ndarray, "Series", "Index")
ListLikeExceptSeriesAndStr = TypeVar(
"ListLikeExceptSeriesAndStr", MutableSequence, np.ndarray, tuple, "Index"
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, the name is awful, but do we have a better way to exclude str from list-likes?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know of any

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

Can you add a test for result_type="reduce", since you now differentiate between the possible values of arg_type ?

Otherwise looks good

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

I Like the additional tests, but all the ones you commented out do work, so we should accept them in typing.

@gandhis1
Copy link
Contributor Author

gandhis1 commented Dec 4, 2022

I've updated the overloads to also handle the scalar value type when a Series[T] is returned. I've also updated the tests to include all cases which return a value.

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

thanks @gandhis1

@Dr-Irv Dr-Irv merged commit d89767d into pandas-dev:main Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants