BUG: Concat typing
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

cc @simonjayhawkins 

Attempting to add typing to results in aggregation.transform. Here, the obj can be a Series but the list passed into concat contain frames. mypy correctly identifies this as an issue, I think concat needs to be typed as FrameOrSeriesUnion for this purpose.

When I try just typing the function definition and not the overload, mypy complains:

    error: Overloaded function implementation does not accept all possible arguments of signature 2  [misc]

so I've changed the type of the overload to FrameOrSeriesUnion as well. I don't believe this is any weaker than the current state because it is an overload.

Labels:
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
mypy fails in the CI check here because concat is called within NDFrame, which is not included in FrameOrSeriesUnion. I think part of the issue might be some odd behavior I'm seeing with Mapping and mypy. Still working through it, marking as a draft.
@rhshadrach https://github.com/pandas-dev/pandas/pull/30852#pullrequestreview-378408089 might help with the backstory for this.

It is worth noting that `FrameOrSeriesUnion` reports errors if concat called from an abstract/base class. And `FrameOrSeries` is a typevar and hence is used to preserve types. Maybe just `NDFrame` is what you require?
Thanks @simonjayhawkins, I've updated the type-hints to use NDFrame. We don't want FrameOrSeriesUnion here because concat can be called from NDFrame and we don't want FrameOrSeries because the argument could be a list of both frames and series.

One other thing I'll note is that while trying to provide various overloads, I ran into [this bug](https://github.com/python/mypy/issues/9451) that seems to impact many uses of overload where the arguments are Mapping or Dict (among other things).
> It is worth noting that `FrameOrSeriesUnion` reports errors if concat called from an abstract/base class

Unrelated to this PR but what is the point of FrameOrSeriesUnion vs just using NDFrame? Just more user-friendly from an API perspective right?
> > It is worth noting that `FrameOrSeriesUnion` reports errors if concat called from an abstract/base class
> 
> Unrelated to this PR but what is the point of FrameOrSeriesUnion vs just using NDFrame? Just more user-friendly from an API perspective right?

@WillAyd the naming was decided in #30582. Looking back through the PR I _think_ both of us prefered a `FrameOrSeries` and `FrameOrSeriesT` naming convention. That's also reason I closed #36100. I didn't want to propagate the current naming convention. https://github.com/pandas-dev/pandas/pull/36100#discussion_r483753222

and I think somewhere in the history before that it was prefered to not have NDFrame in type annotations that might be visible to end users.

I'm +1 on NDFrame and NDFrameT where more appropriate. 
didn't we decide that NDFrame is confusing? 

so am confused by

> @WillAyd the naming was decided in #30582. Looking back through the PR I think both of us prefered a FrameOrSeries and FrameOrSeriesT naming convention. That's also reason I closed #36100. I didn't want to propagate the current naming convention. #36100 (comment)

@WillAyd you are now +1 on this? 
Within the scope of this PR I think it make sense to not try and use the Union type of two objects in the parent. 

Probably fine to have that Union from an API perspective but doesn't need to be forced internally
ok then,

should clarify in _typing on what should be used when for these types.