-
-
Notifications
You must be signed in to change notification settings - Fork 141
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
Conversation
@@ -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" | |||
) |
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.
I agree, the name is awful, but do we have a better way to exclude str
from list-likes?
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.
I don't know of any
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 you add a test for result_type="reduce"
, since you now differentiate between the possible values of arg_type
?
Otherwise looks good
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.
I Like the additional tests, but all the ones you commented out do work, so we should accept them in typing.
I've updated the overloads to also handle the scalar value type when a |
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.
thanks @gandhis1
assert_type()
to assert the type of any return valueIn #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 theresult_type
needs to be accounted for, as well as differentiating between a true list-like,str
, andSeries
in the return value of the function.@Dr-Irv @bashtage @danielroseman