pandas-dev / pandas-stubs

Public type stubs for pandas
BSD 3-Clause "New" or "Revised" License
236 stars 129 forks source link

pd.Series gets converted to pd.TimestampSeries after isinstance check #877

Open tvdboom opened 9 months ago

tvdboom commented 9 months ago

Describe the bug A pd.Series variable changes type after an isinstance check.

To Reproduce

  1. Provide a minimal runnable pandas example that is not properly checked by the stubs.
    df1: pd.Series | pd.DataFrame = pd.Series()
    reveal_type(df1)  # <- As expected
    if isinstance(df1, pd.Series):
    reveal_type(df1)  # <- ???
  2. Indicate which type checker you are using (mypy or pyright). mypy
  3. Show the error message received from that type checker while checking your example.
    note: Revealed type is "Union[pandas.core.series.Series[Any], pandas.core.frame.DataFrame]"
    note: Revealed type is "pandas.core.series.TimestampSeries"

Please complete the following information:

Dr-Irv commented 9 months ago

pyright 1.1.351 gets it right:

issue877.py:4:13 - information: Type of "df1" is "Series[Unknown]"
issue877.py:5:4 - error: Unnecessary isinstance call; "Series[Unknown]" is always an instance of "Series[Unknown]" (reportUnnecessaryIsInstance)
issue877.py:6:17 - information: Type of "df1" is "Series[Unknown]"

I think this is a mypy bug. See https://github.com/python/mypy/issues/15322

I'll keep this open in case mypy ever fixes that issue.

loicdiridollou commented 1 week ago

I am unable to reproduce this one, used the same example as @tvdboom above and this is what mypy returns to me:

===========================================
Beginning: 'Run mypy on 'tests' (using the local stubs) and on the local stubs'
===========================================

tests/test_frame.py:3584: note: Revealed type is "Union[pandas.core.series.Series[Any], pandas.core.frame.DataFrame]"
tests/test_frame.py:3586: note: Revealed type is "pandas.core.series.Series[builtins.float]"

Not sure exactly where it got fixed but it seems like it does not map to TimestampSeries anymore.

Dr-Irv commented 1 week ago

It's still an issue, because what happens now is that it is picking up the first overload for Series.__new__(), which returns a Series[float]. The first reveal_type() showed that the Union contained Series[Any], and you'd expect the same from the second reveal_type().

So mypy still has to address the linked issue.

loicdiridollou commented 1 week ago

Ohh I understand, thanks a lot for all the details!