Make Series.groupby.transform annotation more precise #459
Conversation
| "std", | ||
| "sum", | ||
| "var", | ||
| # Transformation functions |
There was a problem hiding this comment.
So this is another area where I really think a static analysis tool ought to save you from yourself and prevent you from making an illogical call. In this case, we could allow aggregation functions like sum because they work with transform(), however they make no sense to use as a transformation function.
Similar to the last PR, I am allowing it, because pandas permits it and returns a value without erroring. But I'm personally not a big fan of that.
There was a problem hiding this comment.
You could always bring it up as a pandas issue for discussion.
| *args: Any, # Ideally we would use P.args here, but it does not work with engine/engine_kwargs present | ||
| engine: Literal["cython", "numba", None] = ..., | ||
| engine_kwargs: dict[str, Any] | None = ..., | ||
| **kwargs: Any, # Ideally we would use P.kwargs here, but does not work with engine/engine_kwargs present |
There was a problem hiding this comment.
This was working great when I did not have engine and engine_kwargs in there because I could use P.args and P.kwargs as the annotation. However, I'm having trouble getting that working with these additional keyword arguments in there. Any ideas?
There was a problem hiding this comment.
**kwargs: dict[str, Any]. You might need a **, before engine.
There was a problem hiding this comment.
Or maybe place engine and engine_kwargs before *args and **kwargs ???
There was a problem hiding this comment.
Or maybe place
engineandengine_kwargsbefore*argsand**kwargs???
So pyright no longer complains about that, but then the test doesn't work, because it thinks engine and engine_kwargs are positional parameters.
Still investigating...
There was a problem hiding this comment.
Looks like it isn't possible. Here is a similar issue: microsoft/pyright#4189
The relevant part of PEP 612:
Inside of
bar, we would getTypeError: foo() got multiple values for argument 'x'. Requiring these concatenated arguments to be addressed positionally avoids this kind of problem, and simplifies the syntax for spelling these types. Note that this also why we have to reject signatures of the form(*args: P.args, s: str, **kwargs: P.kwargs)(See Concatenating Keyword Parameters for more details).
This is unfortunate. I'll have to revert back to using Any.
|
I'm going to close this until I can dedicate the time to get this to the finish line. |
assert_type()to assert the type of any return value