TYP: clean notations in the wrong place

[jreback]

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

[jreback]

cc @simonjayhawkins @rhshadrach

[simonjayhawkins]

at the very least this should be in pandas._typing. puzzled why we need this at all, as this is the same as FrameOrSeries

IIRC FrameOrSeries is already in use there, so another typevar is needed

[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.

[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

[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.

[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.

[rhshadrach]

@simonjayhawkins Isn't that redundant if the entire file is considered private?

[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)

[simonjayhawkins]

removed the milestone

[jbrockmendel]

FrameOrSeries is no longer used; is OutputFrameOrSeries still needed?

[jbrockmendel]

@rhshadrach i suspect this is closable?