https://github.com/pandas-dev/pandas/blob/master/pandas/core/groupby/groupby.py#L1294
# To track operations that expand dimensions, like ohlc
OutputFrameOrSeries = TypeVar("OutputFrameOrSeries", bound=NDFrame)
at the very least this should be in pandas._typing
. puzzled why we need this at all, as this is the same as FrameOrSeries
Comment From: jreback
cc @simonjayhawkins @rhshadrach
Comment From: simonjayhawkins
at the very least this should be in
pandas._typing
. puzzled why we need this at all, as this is the same asFrameOrSeries
IIRC FrameOrSeries is already in use there, so another typevar is needed
Comment From: rhshadrach
The issue is that FrameOrSeries is bound to the Groupby class. So any method of a subclass cannot use FrameOrSeries freely. For example, a helper method that takes in the result and reindexes it. We'd like the typing to reflect that the argument type to this method is the same as the return type.
Personally, I think the code would be more clear if we added a TypeVar to bind to the GroupBy class and then use FrameOrSeries in addition when needed. In other words, switch the roles of FrameOrSeries and OutputFrameOrSeries (and rename the latter). This would more closely parallel code outside of groupby.
Comment From: simonjayhawkins
Personally, I think the code would be more clear if we added a TypeVar to bind to the GroupBy class and then use FrameOrSeries in addition when needed. In other words, switch the roles of FrameOrSeries and OutputFrameOrSeries (and rename the latter). This would more closely parallel code outside of groupby.
sgtm
Comment From: rhshadrach
@jreback: Does my suggestion above satisfy this issue? If so, maybe we could align on a name/location for the TypeVar. Using GroupByFrameOrSeries
seems a bit verbose to me; I'd suggest GBFrameOrSeries
or GroupByObj
or GroupByData
. From comments above, it seems like it'd be appropriate to add to _typing
with a comment to the effect that it is to only be used to bind to GroupBy.
Comment From: simonjayhawkins
If so, maybe we could align on a name/location for the TypeVar.
from https://github.com/microsoft/pyright/blob/master/docs/typed-libraries.md#best-practices-for-inlined-types
Typically, a TypeVar should be private to the file that declares it, and should therefore begin with an underscore.
Comment From: rhshadrach
@simonjayhawkins Isn't that redundant if the entire file is considered private?
Comment From: simonjayhawkins
IIRC that's the argument why we don't currently add a preceding underscore.
We don't currently have a comprehensive style guide for type annotations, so I would prefer to use something like https://github.com/microsoft/pyright/blob/master/docs/typed-libraries.md#best-practices-for-inlined-types where we don't document our style
(I also like the layout of the google Python style https://google.github.io/styleguide/pyguide.html with a background to why an appropriate style is chosen, I think adopting the style of that styleguide could help revisiting style choices which will normally be subjective)
Comment From: simonjayhawkins
removed the milestone
Comment From: jbrockmendel
FrameOrSeries is no longer used; is OutputFrameOrSeries still needed?