Skip to content

Support for pyright 1.1.310 #716

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 30, 2023
Merged

Support for pyright 1.1.310 #716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8f1a853
Use Self with __new__
Dr-Irv May 25, 2023
ac865c8
change S1 to use Union, add __new__ to test_pandera_generic
Dr-Irv May 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 18 additions & 17 deletions pandas-stubs/_typing.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,25 +310,26 @@ UsecolsArgType: TypeAlias = (
| None
)
# Scratch types for generics

S1 = TypeVar(
"S1",
str,
bytes,
datetime.date,
datetime.time,
bool,
int,
float,
complex,
Dtype,
Timestamp,
Timedelta,
Period,
Interval[int],
Interval[float],
Interval[Timestamp],
Interval[Timedelta],
CategoricalDtype,
bound=str
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See point #3 at microsoft/pyright#5178 (comment) , Recommended to use a Union rather than a constrained type var.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also affect other TypeVars (could be done later)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use S1 = TypeVar("S1", bound=Scalar)? You replaced S1 with Scalar in some places

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

 S1 = TypeVar(
     "S1",
    bound=Scalar
     | Interval[int]
     | Interval[float]
     | Interval[Timestamp]
     | Interval[Timedelta]
    | Dtype
    | Period
    | CategoricalDtype
    | datetime.time,
 )

Seems to work (but Scalar includes a few more types!)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also affect other TypeVars (could be done later)?

Yes, and I created an issue for that #719

| bytes
| datetime.date
| datetime.time
| bool
| int
| float
| complex
| Dtype
| Timestamp
| Timedelta
| Period
| Interval[int]
| Interval[float]
| Interval[Timestamp]
| Interval[Timedelta]
| CategoricalDtype,
)
T1 = TypeVar(
"T1", str, int, np.int64, np.uint64, np.float64, float, np.dtype[np.generic]
Expand Down
11 changes: 6 additions & 5 deletions pandas-stubs/core/frame.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ from pandas.core.window.rolling import (
Rolling,
Window,
)
from typing_extensions import Self
import xarray as xr

from pandas._libs.missing import NAType
Expand Down Expand Up @@ -154,7 +155,7 @@ class _iLocIndexerFrame(_iLocIndexer):
| tuple[IndexType, int]
| tuple[IndexType, IndexType]
| tuple[int, IndexType],
value: S1 | Series | DataFrame | np.ndarray | None,
value: Scalar | Series | DataFrame | np.ndarray | None,
) -> None: ...

class _LocIndexerFrame(_LocIndexer):
Expand Down Expand Up @@ -203,13 +204,13 @@ class _LocIndexerFrame(_LocIndexer):
def __setitem__(
self,
idx: MaskType | StrLike | _IndexSliceTuple | list[ScalarT],
value: S1 | ArrayLike | Series | DataFrame | list | None,
value: Scalar | ArrayLike | Series | DataFrame | list | None,
) -> None: ...
@overload
def __setitem__(
self,
idx: tuple[_IndexSliceTuple, HashableT],
value: S1 | ArrayLike | Series[S1] | list | None,
value: Scalar | ArrayLike | Series | list | None,
) -> None: ...

class DataFrame(NDFrame, OpsMixin):
Expand All @@ -227,7 +228,7 @@ class DataFrame(NDFrame, OpsMixin):
columns: Axes | None = ...,
dtype=...,
copy: _bool = ...,
) -> DataFrame: ...
) -> Self: ...
@overload
def __new__(
cls,
Expand All @@ -236,7 +237,7 @@ class DataFrame(NDFrame, OpsMixin):
columns: Axes,
dtype=...,
copy: _bool = ...,
) -> DataFrame: ...
) -> Self: ...
def __dataframe__(
self, nan_as_null: bool = ..., allow_copy: bool = ...
) -> DataFrameXchg: ...
Expand Down
8 changes: 4 additions & 4 deletions pandas-stubs/core/indexes/base.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class _IndexGetitemMixin(Generic[S1]):
class Index(IndexOpsMixin, PandasObject):
__hash__: ClassVar[None] # type: ignore[assignment]
@overload
def __new__(
def __new__( # type: ignore[misc]
cls,
data: Iterable,
dtype: Literal["int"] | type_t[int] | type_t[np.integer],
Expand All @@ -80,7 +80,7 @@ class Index(IndexOpsMixin, PandasObject):
**kwargs,
) -> _IntIndexType: ...
@overload
def __new__(
def __new__( # type: ignore[misc]
cls,
data: Iterable,
dtype: Literal["float"]
Expand All @@ -93,7 +93,7 @@ class Index(IndexOpsMixin, PandasObject):
**kwargs,
) -> _FloatIndexType: ...
@overload
def __new__(
def __new__( # type: ignore[misc]
cls,
data: Iterable,
dtype: Literal["complex"] | type_t[complex],
Expand All @@ -111,7 +111,7 @@ class Index(IndexOpsMixin, PandasObject):
name=...,
tupleize_cols: bool = ...,
**kwargs,
) -> Index: ...
) -> Self: ...
@property
def str(self) -> StringMethods[Index, MultiIndex]: ...
@property
Expand Down
28 changes: 17 additions & 11 deletions pandas-stubs/core/series.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ from pandas.core.window.rolling import (
)
from typing_extensions import (
Never,
Self,
TypeAlias,
)
import xarray as xr
Expand Down Expand Up @@ -302,16 +303,21 @@ class Series(IndexOpsMixin, NDFrame, Generic[S1]):
name: Hashable | None = ...,
copy: bool = ...,
fastpath: bool = ...,
) -> Series[S1]: ...
) -> Self: ...
@overload
def __new__(
cls,
data: object
| _ListLike
| Series[S1]
| dict[int, S1]
| dict[_str, S1]
| None = ...,
data: Series[S1] | dict[int, S1] | dict[_str, S1] = ...,
index: Axes | None = ...,
dtype=...,
name: Hashable | None = ...,
copy: bool = ...,
fastpath: bool = ...,
) -> Self: ...
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This overload was previously connected to the next one in terms of the data argument, but I split it into two to have it return Self when S1 is known, and to return Series if not.

Copy link
Member

@twoertwein twoertwein May 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the two Self overloads be combined?

edit: Should we relax S1 as we explicitly allow object (which is correct)?

edit2: The following replaces the last three overloads and passes all tests with pyright (but not mypy):

     @overload
     def __new__(
         cls,
         data: Series[S1] | dict[int, S1] | dict[_str, S1] | object= ...,
         index: Axes | None = ...,
         dtype: type[S1] | type | str | None=...,
         name: Hashable = ...,
         copy: bool = ...,
         fastpath: bool = ...,
     ) -> Self: ...

About mypy errors: since they now have more frequent releases, I wouldn't mind making changes that break mypy-compatibility IF 1) there is a long-standing mypy issue, 2) after informing them that we will break compatibility, and 3) if it helps clean pandas-stubs up.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the two Self overloads be combined?

The reason I separated them is that we want to allow people to have an untyped data and specify dtype = type[S1], OR specify a typed data (where S1 can be inferred) and not specify dtype. If we combine them, then it allows any object along with dtype: str and that is an untyped Series. That's where mypy has trouble.

edit: Should we relax S1 as we explicitly allow object (which is correct)?

No. I want to make it that if we know the type and can do something special with it (e.g., Series[Timestamp].__add__(Timedelta)) we differentiate that. object is too wide, so that will correspond to Series[Any] (or in pyright terms, Series[Unknown])

edit2: The following replaces the last three overloads and passes all tests with pyright (but not mypy):

     @overload
     def __new__(
         cls,
         data: Series[S1] | dict[int, S1] | dict[_str, S1] | object= ...,
         index: Axes | None = ...,
         dtype: type[S1] | type | str | None=...,
         name: Hashable = ...,
         copy: bool = ...,
         fastpath: bool = ...,
     ) -> Self: ...

See above reason why I don't want to combine them.

The mypy error is reported here: python/mypy#15322

About mypy errors: since they now have more frequent releases, I wouldn't mind making changes that break mypy-compatibility IF 1) there is a long-standing mypy issue, 2) after informing them that we will break compatibility, and 3) if it helps clean pandas-stubs up.

I'm more hesitant to do that. Most of our bug reports from the community are things people find by using mypy. It is the dominant type checker for now, so we should make sure we handle people's code with it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edit: Should we relax S1 as we explicitly allow object (which is correct)?

No. I want to make it that if we know the type and can do something special with it (e.g., Series[Timestamp].__add__(Timedelta)) we differentiate that. object is too wide, so that will correspond to Series[Any] (or in pyright terms, Series[Unknown])

I didn't mean to use data: object but to simply widen S1 to S1 = TypeVar("S1"), which allows any "object". Wouldn't this still allow special overloads? In either way, this PR is targeted for pyright compatibility and shouldn't depend on that.

@overload
def __new__(
cls,
data: object | _ListLike | None = ...,
index: Axes | None = ...,
dtype=...,
name: Hashable | None = ...,
Expand Down Expand Up @@ -1923,7 +1929,7 @@ class Series(IndexOpsMixin, NDFrame, Generic[S1]):
# ignore needed because of mypy, for using `Never` as type-var.
@overload
def sum(
self: Series[Never], # type: ignore[type-var]
self: Series[Never],
axis: AxisIndex | None = ...,
skipna: _bool | None = ...,
level: None = ...,
Expand Down Expand Up @@ -2008,16 +2014,16 @@ class TimestampSeries(Series[Timestamp]):
# ignore needed because of mypy
@property
def dt(self) -> TimestampProperties: ... # type: ignore[override]
def __add__(self, other: TimedeltaSeries | np.timedelta64) -> TimestampSeries: ... # type: ignore[override]
def __radd__(self, other: TimedeltaSeries | np.timedelta64) -> TimestampSeries: ... # type: ignore[override]
def __add__(self, other: TimedeltaSeries | np.timedelta64 | timedelta) -> TimestampSeries: ... # type: ignore[override]
def __radd__(self, other: TimedeltaSeries | np.timedelta64 | timedelta) -> TimestampSeries: ... # type: ignore[override]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently pyright 1.1.310 had bugs that revealed we were missing some types here

@overload # type: ignore[override]
def __sub__(
self, other: Timestamp | datetime | TimestampSeries
) -> TimedeltaSeries: ...
@overload
def __sub__(
self,
other: Timedelta | TimedeltaSeries | TimedeltaIndex | np.timedelta64,
other: timedelta | TimedeltaSeries | TimedeltaIndex | np.timedelta64,
) -> TimestampSeries: ...
def __mul__(self, other: float | Series[int] | Series[float] | Sequence[float]) -> TimestampSeries: ... # type: ignore[override]
def __truediv__(self, other: float | Series[int] | Series[float] | Sequence[float]) -> TimestampSeries: ... # type: ignore[override]
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ mypy = "1.3.0"
pandas = "2.0.1"
pyarrow = ">=10.0.1"
pytest = ">=7.1.2"
pyright = "<= 1.1.308"
pyright = ">= 1.1.310"
poethepoet = ">=0.16.5"
loguru = ">=0.6.0"
typing-extensions = ">=4.4.0"
Expand Down
6 changes: 3 additions & 3 deletions tests/test_frame.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2598,15 +2598,15 @@ def test_align() -> None:
columns=["A", "B", "C"],
)

s0 = pd.Series(data={1: "1", 3: "3", 5: "5"})
s0 = pd.Series(data={0: "1", 3: "3", 5: "5"})
aligned_df0, aligned_s0 = df0.align(s0, axis="index")
check(assert_type(aligned_df0, pd.DataFrame), pd.DataFrame)
check(assert_type(aligned_s0, pd.Series), pd.Series)
check(assert_type(aligned_s0, "pd.Series[str]"), pd.Series, str)

s1 = pd.Series(data={"A": "A", "D": "D"})
aligned_df0, aligned_s1 = df0.align(s1, axis="columns")
check(assert_type(aligned_df0, pd.DataFrame), pd.DataFrame)
check(assert_type(aligned_s1, pd.Series), pd.Series)
check(assert_type(aligned_s1, "pd.Series[str]"), pd.Series, str)

df1 = pd.DataFrame(
data=np.array(
Expand Down
7 changes: 5 additions & 2 deletions tests/test_series.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from pandas.core.window import ExponentialMovingWindow
import pytest
from typing_extensions import (
Self,
TypeAlias,
assert_type,
)
Expand Down Expand Up @@ -1604,12 +1605,14 @@ def test_pandera_generic() -> None:
T = TypeVar("T")

class MySeries(pd.Series, Generic[T]):
...
def __new__(cls, *args, **kwargs) -> Self:
return object.__new__(cls)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes pyright not complain.


def func() -> MySeries[float]:
return MySeries[float]([1, 2, 3])

func()
result = func()
assert result.iloc[1] == 2
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified the test to make sure that defining __new__() didn't change the result.



def test_change_to_dict_return_type() -> None:
Expand Down