TYP: Overload concat #41184

pckSF · 2021-04-27T19:10:01Z

Closes part of Function overloads MarcoGorelli/pyladieslondon-sprints#29
Adds typing.overload to: concat from https://github.com/pandas-dev/pandas/blob/master/pandas/core/reshape/concat.py

`mypy` Setup and Sutput

# t.py
import pandas as pd
from pandas._typing import Axis
from pandas.core.generic import NDFrame

axis: Axis

ndf1: NDFrame
ndf2: NDFrame

s1 = pd.Series
s2 = pd.Series

df1 = pd.DataFrame
df2 = pd.DataFrame

reveal_type(pd.concat([s1, s2]))
reveal_type(pd.concat([s1, s2], axis=0))
reveal_type(pd.concat([s1, s2], axis=1))
reveal_type(pd.concat([s1, s2], axis="index"))
reveal_type(pd.concat([s1, s2], axis="columns"))

reveal_type(pd.concat([df1, df2]))
reveal_type(pd.concat([df1, df2], axis=0))
reveal_type(pd.concat([df1, df2], axis=1))
reveal_type(pd.concat([df1, df2], axis="index"))
reveal_type(pd.concat([df1, df2], axis="columns"))

reveal_type(pd.concat([ndf1, ndf2]))
reveal_type(pd.concat([ndf1, ndf2], axis=0))
reveal_type(pd.concat([ndf1, ndf2], axis=1))
reveal_type(pd.concat([ndf1, ndf2], axis="index"))
reveal_type(pd.concat([ndf1, ndf2], axis="columns"))

which results in the following mypy output:

$ mypy t.py
  t.py:16: note: Revealed type is 'pandas.core.series.Series'
  t.py:17: note: Revealed type is 'pandas.core.series.Series'
  t.py:18: note: Revealed type is 'pandas.core.frame.DataFrame'
  t.py:19: note: Revealed type is 'pandas.core.series.Series'
  t.py:20: note: Revealed type is 'pandas.core.frame.DataFrame'
  t.py:22: note: Revealed type is 'pandas.core.frame.DataFrame'
  t.py:23: note: Revealed type is 'pandas.core.frame.DataFrame'
  t.py:24: note: Revealed type is 'pandas.core.frame.DataFrame'
  t.py:25: note: Revealed type is 'pandas.core.frame.DataFrame'
  t.py:26: note: Revealed type is 'pandas.core.frame.DataFrame'
  t.py:28: note: Revealed type is 'Union[pandas.core.frame.DataFrame, pandas.core.series.Series]'
  t.py:29: note: Revealed type is 'Union[pandas.core.frame.DataFrame, pandas.core.series.Series]'
  t.py:30: note: Revealed type is 'pandas.core.frame.DataFrame'
  t.py:31: note: Revealed type is 'Union[pandas.core.frame.DataFrame, pandas.core.series.Series]'
  t.py:32: note: Revealed type is 'pandas.core.frame.DataFrame'

pckSF · 2021-04-27T19:13:44Z

The only issue left seems to be

$ mypy t.py
...
  pandas/core/reshape/concat.py:58: error: Overloaded function signatures 1 and 3 overlap with incompatible return types  [misc]
...

but signature 1 has objs: Iterable[DataFrame] | Mapping[Hashable, DataFrame], while signature 3 has objs: Iterable[Series] | Mapping[Hashable, Series], so it should not overlap?

I played around with it but could not get the desired mypy t.py output without also encountering this error :(

MarcoGorelli · 2021-04-27T20:33:55Z

Thanks for looking into this - I also wouldn't have expected them to overlap, so I'm not sure why this is happening, but I'll look into it later

pckSF · 2021-05-01T19:47:46Z

Updated overloads as I just realized that a passed objs of type NDFrame should also return a DataFrame if axis = Literal[1, "columns"] and only else a Union[pandas.core.frame.DataFrame, pandas.core.series.Series].

MarcoGorelli · 2021-05-03T10:33:20Z

I can't make sense of the mypy error you're getting:

from __future__ import annotations
from typing import overload, Iterable, Mapping, Hashable

@overload
def foo(myarg: Iterable[str] | Mapping[Hashable, str]) -> str: ...

@overload
def foo(myarg: Iterable[bool] | Mapping[Hashable, bool]) -> bool: ...

def foo(myarg) -> str | bool: ...

$ mypy t.py
t.py:5: error: Overloaded function signatures 1 and 2 overlap with incompatible return types  [misc]
Found 1 error in 1 file (checked 1 source file)

I'll ask on StackOverflow, then open an issue in mypy if I don't get a response - will get back to you as soon as I have an answer!

MarcoGorelli · 2021-05-03T12:08:20Z

OK, I see the issue: it's because Mapping[TypeA, TypeB] is an Iterable[TypeA]. E.g. the following type checks fine:

from typing import Iterable


a: Iterable[str]
a = {'foo': 0}

So...gonna have to think about what to do about this

e.g.

from typing import Hashable
from pandas import DataFrame


a: Hashable
a = DataFrame()

type checks fine.

So, if you pass a mapping in which the keys are Series, then it could match Mapping[Hashable, DataFrame], but also Iterable[Series]

github-actions · 2021-06-22T00:03:09Z

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

MarcoGorelli · 2021-06-22T07:36:53Z

This is pending on #41283 being merged first

simonjayhawkins · 2021-07-03T10:28:06Z

This is pending on #41283 being merged first

#41283 is now merged.

MarcoGorelli · 2021-07-11T13:26:08Z

Hey @pckSF - if you fetch and merge upstream/master, then I'd expect the mypy issue to be fixed

pckSF · 2021-07-11T19:46:03Z

Hey @pckSF - if you fetch and merge upstream/master, then I'd expect the mypy issue to be fixed

Done, and the error is gone

MarcoGorelli

can you fixup the remaining lint issues?

mypy --version
mypy 0.910
Performing static analysis using mypy
pandas/core/reshape/reshape.py:1053: error: Redundant cast to "DataFrame"  [redundant-cast]
pandas/core/reshape/melt.py:139: error: Redundant cast to "Series"  [redundant-cast]
pandas/core/groupby/generic.py:312: error: unused "type: ignore" comment
pandas/core/groupby/generic.py:551: error: unused "type: ignore" comment
Found 4 errors in 3 files (checked 1299 source files)
Performing static analysis using mypy DONE
Error: Process completed with exit code 1.

pandas/core/reshape/concat.py

MarcoGorelli · 2021-07-18T09:39:12Z

nice - is this ready for review?

pckSF · 2021-07-18T12:02:24Z

nice - is this ready for review?

Thanks for the reminder, I always forget that I have them in preview...

pandas/core/reshape/concat.py

MarcoGorelli · 2021-07-19T20:20:17Z

Nice, looks good - the typing failure is unrelated, might need to merge again when that's fixed on master

simonjayhawkins · 2021-07-19T20:28:37Z

Nice, looks good - the typing failure is unrelated, might need to merge again when that's fixed on master

just merged #42614

MarcoGorelli

I think a couple of these might be unnecessary, because the return type is always the same if axis is 1/"columns"

Could you check that you check the same result by removing them? (if not I can do it later in the week)

pandas/core/reshape/concat.py

MarcoGorelli · 2021-07-20T20:14:52Z

Performing static analysis using mypy
pandas/core/arrays/categorical.py:2314: error: Incompatible types in assignment (expression has type "List[str]", variable has type "Index")  [assignment]
pandas/core/generic.py:5752: error: Incompatible return value type (got "DataFrame", expected "FrameOrSeries")  [return-value]
pandas/core/generic.py:6115: error: Incompatible return value type (got "DataFrame", expected "FrameOrSeries")  [return-value]
Found 3 errors in 2 files (checked 1301 source files)
Performing static analysis using mypy DONE

🤔 maybe those overloads weren't redundant...I'm not sure why though, I'll look into it

pckSF · 2021-07-20T22:27:48Z

Performing static analysis using mypy
pandas/core/arrays/categorical.py:2314: error: Incompatible types in assignment (expression has type "List[str]", variable has type "Index")  [assignment]
pandas/core/generic.py:5752: error: Incompatible return value type (got "DataFrame", expected "FrameOrSeries")  [return-value]
pandas/core/generic.py:6115: error: Incompatible return value type (got "DataFrame", expected "FrameOrSeries")  [return-value]
Found 3 errors in 2 files (checked 1301 source files)
Performing static analysis using mypy DONE

🤔 maybe those overloads weren't redundant...I'm not sure why though, I'll look into it

Question regarding pandas/core/generic.py:5752: error: Incompatible return value type (got "DataFrame", expected "FrameOrSeries") [return-value]:

    def astype(
        self: FrameOrSeries, dtype, copy: bool_t = True, errors: str = "raise"
    ) -> FrameOrSeries:
    ...
    
    ...
    result = concat(results, axis=1, copy=False)
    result.columns = self.columns
    return result

given that it uses concat along axis=1 shoudn't the returnd value be a DataFrame?
(Similar case for pandas/core/generic.py:6115: error: Incompatible return value type (got "DataFrame", expected "FrameOrSeries") [return-value])

MarcoGorelli · 2021-07-24T10:39:54Z

If I do

diff --git a/pandas/core/generic.py b/pandas/core/generic.py
index 3b8458fac2..db6f213b3b 100644
--- a/pandas/core/generic.py
+++ b/pandas/core/generic.py
@@ -5747,7 +5747,9 @@ class NDFrame(PandasObject, indexing.IndexingMixin):
             return self.copy()
 
         # GH 19920: retain column metadata after concat
+        reveal_type(results)
         result = concat(results, axis=1, copy=False)
+        reveal_type(result)
         result.columns = self.columns
         return result

then I get

$ mypy pandas
pandas/core/arrays/categorical.py:2318: error: Incompatible types in assignment (expression has type "List[str]", variable has type "Index")  [assignment]
pandas/core/generic.py:5750: note: Revealed type is "builtins.list[Any]"
pandas/core/generic.py:5752: note: Revealed type is "pandas.core.frame.DataFrame"
pandas/core/generic.py:5754: error: Incompatible return value type (got "DataFrame", expected "FrameOrSeries")  [return-value]
pandas/core/generic.py:6117: error: Incompatible return value type (got "DataFrame", expected "FrameOrSeries")  [return-value]
Found 3 errors in 2 files (checked 1299 source files)

, so the revealed type at pandas/core/generic.py:5752 looks correct. However, if I roll back to the previous commit, then that is revealed to be Any:

$ mypy pandas
pandas/core/generic.py:5750: note: Revealed type is "builtins.list[Any]"
pandas/core/generic.py:5752: note: Revealed type is "Any"

So, it worked in the previous commit because the type checker was being disabled on the result.

Likewise, on master:

$ mypy pandas
pandas/core/generic.py:5750: note: Revealed type is "builtins.list[Any]"
pandas/core/generic.py:5752: note: Revealed type is "Any"

So, what you have in the current commit seems correct, although I can't figure out why in your previous commit the type of result was not inferred correctly

MarcoGorelli · 2021-07-24T11:57:17Z

OK, looks like we're running into python/mypy#8354

So, I'd suggest you do something like

diff --git a/pandas/core/generic.py b/pandas/core/generic.py
index 3b8458fac2..6205463aad 100644
--- a/pandas/core/generic.py
+++ b/pandas/core/generic.py
@@ -5749,7 +5749,8 @@ class NDFrame(PandasObject, indexing.IndexingMixin):
         # GH 19920: retain column metadata after concat
         result = concat(results, axis=1, copy=False)
         result.columns = self.columns
-        return result
+        # https://github.com/python/mypy/issues/8354
+        return cast(FrameOrSeries, result)
 
     @final
     def copy(self: FrameOrSeries, deep: bool_t = True) -> FrameOrSeries:
@@ -6112,7 +6113,8 @@ class NDFrame(PandasObject, indexing.IndexingMixin):
                 for col_name, col in self.items()
             ]
             if len(results) > 0:
-                return concat(results, axis=1, copy=False)
+                # https://github.com/python/mypy/issues/8354
+                return cast(FrameOrSeries, concat(results, axis=1, copy=False))
             else:
                 return self.copy()

The other issue seems correct though, for that I'd suggest

diff --git a/pandas/core/arrays/categorical.py b/pandas/core/arrays/categorical.py
index 3fdb52a73d..57e7c0af52 100644
--- a/pandas/core/arrays/categorical.py
+++ b/pandas/core/arrays/categorical.py
@@ -2315,7 +2315,7 @@ class Categorical(NDArrayBackedExtensionArray, PandasObject, ObjectStringArrayMi
         from pandas.core.reshape.concat import concat
 
         result = concat([counts, freqs], axis=1)
-        result.columns = ["counts", "freqs"]
+        result.columns = Index(["counts", "freqs"])
         result.index.name = "categories"
 
         return result

…concat

MarcoGorelli · 2021-07-26T07:05:18Z

thanks - now the CI is failing because of #42716 , once that's fixed I'd like to think that this PR will be fine

MarcoGorelli · 2021-07-31T09:39:21Z

thanks - now the CI is failing because of #42716 , once that's fixed I'd like to think that this PR will be fine

should be good to fetch/merge upstream/master now

MarcoGorelli · 2021-08-06T11:08:13Z

pandas/core/arrays/categorical.py

@@ -2311,7 +2311,7 @@ def describe(self):
        from pandas.core.reshape.concat import concat

        result = concat([counts, freqs], axis=1)
-        result.columns = ["counts", "freqs"]
+        result.columns = Index(["counts", "freqs"])


you'll need to import Index in/after line 2311 for this to work

MarcoGorelli

Looks good to me

…-reshape-concat

MarcoGorelli · 2021-08-21T18:12:25Z

cc @simonjayhawkins in case you have comments

MarcoGorelli · 2021-08-22T14:00:35Z

Thanks @pckSF !

* TYP: Overload concat * Fix NDFrame columns overload * Fix FrameOrSeries * Fix redundant casts * Import Axis outside of TYPE_CHECKING * Remove redundant overloads * Mark mypy/issues/8354 return type issues * Fix missing imports * noop Co-authored-by: Marco Gorelli <[email protected]>

TYP: Overload concat

c37272d

simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Apr 28, 2021

Fix NDFrame columns overload

7bf21ec

MarcoGorelli self-requested a review May 3, 2021 10:17

MarcoGorelli mentioned this pull request May 3, 2021

TYP Series and DataFrame currently type-check as hashable #41282

Closed

simonjayhawkins added the Reshaping Concat, Merge/Join, Stack/Unstack, Explode label May 22, 2021

github-actions bot added the Stale label Jun 22, 2021

pckSF added 2 commits July 11, 2021 20:30

Merge upstream/master as required pandas-dev#41283 was merged

cc24c9a

Fix FrameOrSeries

ee491ac

MarcoGorelli requested changes Jul 16, 2021

View reviewed changes

pandas/core/reshape/concat.py Outdated Show resolved Hide resolved

Fix redundant casts

29d3da4

pckSF marked this pull request as ready for review July 18, 2021 12:02

MarcoGorelli self-requested a review July 18, 2021 12:05

lithomas1 removed the Stale label Jul 18, 2021

simonjayhawkins reviewed Jul 19, 2021

View reviewed changes

pandas/core/reshape/concat.py Outdated Show resolved Hide resolved

simonjayhawkins added this to the 1.4 milestone Jul 19, 2021

Import Axis outside of TYPE_CHECKING

93143b4

MarcoGorelli requested changes Jul 19, 2021

View reviewed changes

pandas/core/reshape/concat.py Outdated Show resolved Hide resolved

pandas/core/reshape/concat.py Outdated Show resolved Hide resolved

Remove redundant overloads

8783214

MarcoGorelli self-requested a review July 20, 2021 08:33

pckSF added 2 commits July 26, 2021 00:01

Mark mypy/issues/8354 return type issues

eacfe5b

Merge remote-tracking branch 'upstream/master' into overload-reshape-…

376994f

…concat

pckSF added 2 commits August 3, 2021 13:29

Update: Merge Master

245067c

Fetch/Merge upstream/master

34b925f

MarcoGorelli requested changes Aug 6, 2021

View reviewed changes

pckSF added 2 commits August 11, 2021 15:09

Fix missing imports

1f71cef

Update changes from master

d4130cb

MarcoGorelli self-requested a review August 12, 2021 11:17

MarcoGorelli approved these changes Aug 13, 2021

View reviewed changes

MarcoGorelli added 2 commits August 21, 2021 17:38

Merge remote-tracking branch 'upstream/master' into pr/pckSF-overload…

f7f7978

…-reshape-concat

noop

8d523c3

MarcoGorelli merged commit cbec30e into pandas-dev:master Aug 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TYP: Overload concat #41184

TYP: Overload concat #41184

pckSF commented Apr 27, 2021 •

edited

Loading

pckSF commented Apr 27, 2021 •

edited

Loading

MarcoGorelli commented Apr 27, 2021

pckSF commented May 1, 2021

MarcoGorelli commented May 3, 2021

MarcoGorelli commented May 3, 2021 •

edited

Loading

github-actions bot commented Jun 22, 2021

MarcoGorelli commented Jun 22, 2021

simonjayhawkins commented Jul 3, 2021

MarcoGorelli commented Jul 11, 2021

pckSF commented Jul 11, 2021

MarcoGorelli left a comment

MarcoGorelli commented Jul 18, 2021

pckSF commented Jul 18, 2021

MarcoGorelli commented Jul 19, 2021

simonjayhawkins commented Jul 19, 2021

MarcoGorelli left a comment •

edited

Loading

MarcoGorelli commented Jul 20, 2021

pckSF commented Jul 20, 2021

MarcoGorelli commented Jul 24, 2021

MarcoGorelli commented Jul 24, 2021 •

edited

Loading

MarcoGorelli commented Jul 26, 2021

MarcoGorelli commented Jul 31, 2021

MarcoGorelli Aug 6, 2021 •

edited

Loading

MarcoGorelli left a comment

MarcoGorelli commented Aug 21, 2021

MarcoGorelli commented Aug 22, 2021

TYP: Overload concat #41184

TYP: Overload concat #41184

Conversation

pckSF commented Apr 27, 2021 • edited Loading

mypy Setup and Sutput

pckSF commented Apr 27, 2021 • edited Loading

MarcoGorelli commented Apr 27, 2021

pckSF commented May 1, 2021

MarcoGorelli commented May 3, 2021

MarcoGorelli commented May 3, 2021 • edited Loading

github-actions bot commented Jun 22, 2021

MarcoGorelli commented Jun 22, 2021

simonjayhawkins commented Jul 3, 2021

MarcoGorelli commented Jul 11, 2021

pckSF commented Jul 11, 2021

MarcoGorelli left a comment

Choose a reason for hiding this comment

MarcoGorelli commented Jul 18, 2021

pckSF commented Jul 18, 2021

MarcoGorelli commented Jul 19, 2021

simonjayhawkins commented Jul 19, 2021

MarcoGorelli left a comment • edited Loading

Choose a reason for hiding this comment

MarcoGorelli commented Jul 20, 2021

pckSF commented Jul 20, 2021

MarcoGorelli commented Jul 24, 2021

MarcoGorelli commented Jul 24, 2021 • edited Loading

MarcoGorelli commented Jul 26, 2021

MarcoGorelli commented Jul 31, 2021

MarcoGorelli Aug 6, 2021 • edited Loading

Choose a reason for hiding this comment

MarcoGorelli left a comment

Choose a reason for hiding this comment

MarcoGorelli commented Aug 21, 2021

MarcoGorelli commented Aug 22, 2021

pckSF commented Apr 27, 2021 •

edited

Loading

`mypy` Setup and Sutput

pckSF commented Apr 27, 2021 •

edited

Loading

MarcoGorelli commented May 3, 2021 •

edited

Loading

MarcoGorelli left a comment •

edited

Loading

MarcoGorelli commented Jul 24, 2021 •

edited

Loading

MarcoGorelli Aug 6, 2021 •

edited

Loading