CLN: Replace FrameOrSeriesUnion annotation by DataFrame | Series
State: closed

- [x] closes #40876
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry


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

Comments:
@github-actions pre-commit
Hi @MarcoGorelli, would you mind taking a look at my last commit, I don't really know why the particular test is failing. 

Thank you.
> Hi @MarcoGorelli, would you mind taking a look at my last commit, I don't really know why the particular test is failing.
> 
> Thank you.

see https://github.com/pandas-dev/pandas/pull/41955#pullrequestreview-682308945 for the mypy failures.

There is also a pre-commit failure. see https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit
I fixed all the mypy errors from the prev. commit in this commit. Should I run @github-actions pre-commit like it says in the documentation?
> I fixed all the mypy errors from the prev. commit in this commit. 

They're still failing, if you click "details" you'll see the failures:

![Screenshot 2021-06-15 at 16 10 28](https://user-images.githubusercontent.com/33491632/122078147-464b8200-cdf4-11eb-86e1-836d018e1e68.png)

> Should I run @github-actions pre-commit like it says in the documentation?

This won't fix the `flake8` errors, you'll probably need to run it locally before committing
Hi, I can't seem to understand these pre-commit errors. Are they formatting related?
Hey @Swanand01 - yeah, see [here](https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#pre-commit) for how to run them

Seems like this is on the right track anyway!
Hi @MarcoGorelli , all checks pass now. Thank you.
Hi, how will this PR be merged? Do I have to close this PR now?
> Hi, how will this PR be merged? Do I have to close this PR now?

no do not close. other reviewers could either request changes, add their approval or merge this.
> > Hi, how will this PR be merged? Do I have to close this PR now?
> 
> no do not close. other reviewers could either request changes, add their approval or merge this.

Okay thanks.
Thanks @Swanand01 
@simonjayhawkins I thought we discussed in our monthly dev meeting to _not_ do this change because you lose the typeahead feature in editors.  I.e., you can do typeahead for `FrameOrSeriesUnion` but not `DataFrame | Series`
@Dr-Irv my understanding was that that conversation in the dev meeting was spurred from the fact that it looked like this PR had gone stale and it was marked for 1.3. In light of that, and of the typeahead issue you brought up, it didn't seem worth pushing it though

Once activity resumed, though, it seemed fine to stick with it. Perhaps I should've pinged you to check, sorry about that

No objection to reverting (or re-discussing in the next dev meeting) if you wish, but I do think this is fine to keep. An unexpected benefit is that it's helped make it clear that there are cases when it's redundant to have `| Series`, e.g. when there's `AnyArrayLike`.
Plus, I still think it's more explicit than `FrameOrSeriesUnion` (which can be confused with `FrameOrSeries`) and that this outweights having a shorter typeahead
> but I do think this is fine to keep. An unexpected benefit is that it's helped make it clear that there are cases when it's redundant to have `| Series`, e.g. when there's `AnyArrayLike`.
> Plus, I still think it's more explicit than `FrameOrSeriesUnion` (which can be confused with `FrameOrSeries`) and that this outweights having a shorter typeahead

agreed. and with #42367 removing the redundant typevars, things become clearer still.