CLN: Replace FrameOrSeriesUnion annotation by DataFrame | Series #41955

Swanand01 · 2021-06-11T18:16:38Z

closes STYLE, TYP frame-or-series-union check no longer relevant #40876
tests added / passed
Ensure all linting tests pass, see here for how to run them
whatsnew entry

Swanand01 · 2021-06-12T07:57:39Z

@github-actions pre-commit

MarcoGorelli

 pandas/plotting/_matplotlib/timeseries.py:212:35: F821 undefined name 'DataFrame'
pandas/io/formats/info.py:112:23: F821 undefined name 'Series'
pandas/io/formats/info.py:415:35: F821 undefined name 'Series'
pandas/core/window/expanding.py:590:16: F821 undefined name 'DataFrame'
pandas/core/window/expanding.py:590:28: F821 undefined name 'Series'
pandas/core/window/expanding.py:655:16: F821 undefined name 'DataFrame'
pandas/core/window/expanding.py:655:28: F821 undefined name 'Series'
pandas/core/strings/accessor.py:2314:10: F821 undefined name 'DataFrame'
pandas/core/strings/accessor.py:2314:22: F821 undefined name 'Series'
pandas/plotting/_matplotlib/tools.py:56:15: F821 undefined name 'DataFrame'
pandas/plotting/_matplotlib/tools.py:56:27: F821 undefined name 'Series'
pandas/core/window/ewm.py:514:16: F821 undefined name 'DataFrame'
pandas/core/window/ewm.py:514:28: F821 undefined name 'Series'
pandas/core/window/ewm.py:581:16: F821 undefined name 'DataFrame'
pandas/core/window/ewm.py:581:28: F821 undefined name 'Series'
pandas/core/util/hashing.py:77:18: F821 undefined name 'DataFrame'

For such files you should import Series and DataFrame. If you need to avoid a circular import, you can do like they do here:

pandas/pandas/compat/pickle_compat.py

Lines 26 to 30 in 8f70ed5

    
           if TYPE_CHECKING: 
        
               from pandas import ( 
        
                   DataFrame, 
        
                   Series, 
        
               )

…eries-check

Swanand01 · 2021-06-15T12:36:57Z

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.

simonjayhawkins · 2021-06-15T14:22:13Z

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 #41955 (review) 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

Swanand01 · 2021-06-15T14:51:20Z

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?

MarcoGorelli · 2021-06-15T15:11:58Z

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:

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

This reverts commit 1d610ca.

This reverts commit 6bbe77e.

Swanand01 · 2021-06-19T17:18:56Z

Hi, I can't seem to understand these pre-commit errors. Are they formatting related?

MarcoGorelli · 2021-06-19T18:22:16Z

Hey @Swanand01 - yeah, see here for how to run them

Seems like this is on the right track anyway!

Swanand01 · 2021-06-20T17:14:16Z

Hi @MarcoGorelli , all checks pass now. Thank you.

MarcoGorelli

Looks good to me, thanks for having stuck with this!

Swanand01 · 2021-06-21T11:37:02Z

Hi, how will this PR be merged? Do I have to close this PR now?

simonjayhawkins · 2021-06-21T11:43:35Z

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.

Swanand01 · 2021-06-21T11:48:20Z

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.

simonjayhawkins

Thanks @Swanand01 lgtm. just a couple of suggestions.

simonjayhawkins · 2021-07-03T17:42:44Z

pandas/core/frame.py

@@ -1362,7 +1361,7 @@ def dot(self, other: Series) -> Series:
    def dot(self, other: DataFrame | Index | ArrayLike) -> DataFrame:
        ...

-    def dot(self, other: AnyArrayLike | FrameOrSeriesUnion) -> FrameOrSeriesUnion:
+    def dot(self, other: AnyArrayLike | DataFrame | Series) -> DataFrame | Series:


ooh, we should add a check for this

EDIT

#42359

ok. we can merge this and let the check fix this

simonjayhawkins · 2021-07-03T17:43:25Z

pandas/core/frame.py

@@ -1478,13 +1477,13 @@ def __matmul__(self, other: Series) -> Series:

    @overload
    def __matmul__(
-        self, other: AnyArrayLike | FrameOrSeriesUnion
-    ) -> FrameOrSeriesUnion:
+        self, other: AnyArrayLike | DataFrame | Series


same (and already included in previous overload anyway)

simonjayhawkins · 2021-07-03T17:43:33Z

pandas/core/frame.py

        ...

    def __matmul__(
-        self, other: AnyArrayLike | FrameOrSeriesUnion
-    ) -> FrameOrSeriesUnion:
+        self, other: AnyArrayLike | DataFrame | Series


simonjayhawkins · 2021-07-03T18:05:34Z

pandas/core/frame.py

@@ -1362,7 +1361,7 @@ def dot(self, other: Series) -> Series:
    def dot(self, other: DataFrame | Index | ArrayLike) -> DataFrame:
        ...

-    def dot(self, other: AnyArrayLike | FrameOrSeriesUnion) -> FrameOrSeriesUnion:
+    def dot(self, other: AnyArrayLike | DataFrame | Series) -> DataFrame | Series:


ok. we can merge this and let the check fix this

simonjayhawkins · 2021-07-04T10:07:08Z

Thanks @Swanand01

Dr-Irv · 2021-07-04T16:11:57Z

@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

MarcoGorelli · 2021-07-04T17:40:19Z

@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

simonjayhawkins · 2021-07-04T17:47:25Z

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.

CLN: Replace FrameOrSeriesUnion annotation by DataFrame | Series

7e0bda3

MarcoGorelli suggested changes Jun 12, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/master' into remove-frame-or-s…

896b29a

…eries-check

Swanand01 and others added 6 commits June 19, 2021 18:03

Import fixes

6bbe77e

Update ewm.py

1d610ca

Revert "Update ewm.py"

8bee21f

This reverts commit 1d610ca.

Revert "Import fixes"

76d0dbf

This reverts commit 6bbe77e.

Circular imports fix

8ffca34

Update timeseries.py

7e06259

Swanand01 added 3 commits June 20, 2021 20:56

Update rolling.py

dc70410

Update timeseries.py

53acbbf

Update style_render.py

28bd825

lithomas1 added Code Style Code style, linting, code_checks Clean Typing type annotations, mypy/pyright type checking and removed Code Style Code style, linting, code_checks labels Jun 20, 2021

MarcoGorelli approved these changes Jun 20, 2021

View reviewed changes

simonjayhawkins reviewed Jul 3, 2021

View reviewed changes

MarcoGorelli mentioned this pull request Jul 3, 2021

STYLE disallow Series | AnyArrayLike #42359

Closed

simonjayhawkins approved these changes Jul 3, 2021

View reviewed changes

simonjayhawkins added this to the 1.4 milestone Jul 4, 2021

simonjayhawkins merged commit ed2af7e into pandas-dev:master Jul 4, 2021

simonjayhawkins mentioned this pull request Jul 4, 2021

TYP: fix reportInvalidTypeVarUse from pyright #42367

Merged

simonjayhawkins mentioned this pull request Feb 23, 2023

TYP: FrameOrSeriesUnion #36475

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLN: Replace FrameOrSeriesUnion annotation by DataFrame | Series #41955

CLN: Replace FrameOrSeriesUnion annotation by DataFrame | Series #41955

Swanand01 commented Jun 11, 2021 •

edited by simonjayhawkins

Loading

Swanand01 commented Jun 12, 2021

MarcoGorelli left a comment

Swanand01 commented Jun 15, 2021

simonjayhawkins commented Jun 15, 2021

Swanand01 commented Jun 15, 2021

MarcoGorelli commented Jun 15, 2021

Swanand01 commented Jun 19, 2021

MarcoGorelli commented Jun 19, 2021

Swanand01 commented Jun 20, 2021

MarcoGorelli left a comment

Swanand01 commented Jun 21, 2021

simonjayhawkins commented Jun 21, 2021

Swanand01 commented Jun 21, 2021

simonjayhawkins left a comment

simonjayhawkins Jul 3, 2021

MarcoGorelli Jul 3, 2021 •

edited

Loading

simonjayhawkins Jul 3, 2021

simonjayhawkins Jul 3, 2021

simonjayhawkins Jul 3, 2021

simonjayhawkins Jul 3, 2021

simonjayhawkins commented Jul 4, 2021

Dr-Irv commented Jul 4, 2021

MarcoGorelli commented Jul 4, 2021

simonjayhawkins commented Jul 4, 2021

	def dot(self, other: AnyArrayLike \| DataFrame \| Series) -> DataFrame \| Series:
	def dot(self, other: AnyArrayLike \| DataFrame) -> DataFrame \| Series:

CLN: Replace FrameOrSeriesUnion annotation by DataFrame | Series #41955

CLN: Replace FrameOrSeriesUnion annotation by DataFrame | Series #41955

Conversation

Swanand01 commented Jun 11, 2021 • edited by simonjayhawkins Loading

Swanand01 commented Jun 12, 2021

MarcoGorelli left a comment

Choose a reason for hiding this comment

Swanand01 commented Jun 15, 2021

simonjayhawkins commented Jun 15, 2021

Swanand01 commented Jun 15, 2021

MarcoGorelli commented Jun 15, 2021

Swanand01 commented Jun 19, 2021

MarcoGorelli commented Jun 19, 2021

Swanand01 commented Jun 20, 2021

MarcoGorelli left a comment

Choose a reason for hiding this comment

Swanand01 commented Jun 21, 2021

simonjayhawkins commented Jun 21, 2021

Swanand01 commented Jun 21, 2021

simonjayhawkins left a comment

Choose a reason for hiding this comment

simonjayhawkins Jul 3, 2021

Choose a reason for hiding this comment

MarcoGorelli Jul 3, 2021 • edited Loading

Choose a reason for hiding this comment

EDIT

simonjayhawkins Jul 3, 2021

Choose a reason for hiding this comment

simonjayhawkins Jul 3, 2021

Choose a reason for hiding this comment

simonjayhawkins Jul 3, 2021

Choose a reason for hiding this comment

simonjayhawkins Jul 3, 2021

Choose a reason for hiding this comment

simonjayhawkins commented Jul 4, 2021

Dr-Irv commented Jul 4, 2021

MarcoGorelli commented Jul 4, 2021

simonjayhawkins commented Jul 4, 2021

Swanand01 commented Jun 11, 2021 •

edited by simonjayhawkins

Loading

MarcoGorelli Jul 3, 2021 •

edited

Loading