Skip to content
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.

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

Issue1098 #1172

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Issue1098 #1172

wants to merge 4 commits into from

Conversation

shirzady1934
Copy link
Contributor

This PR addresses issue #1098. I have fixed the add method and included a test case for it. However, I'm facing some challenges resolving the mul method due to conflicts with other test cases that are causing failures. If you permit me to modify or remove some of these test cases, I can work on fixing the issue in a separate PR.

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

This won't work. Consider this example:

    s = pd.Series([1, 2, 3])
    c = 1.2
    reveal_type(s + c)

The revealed type with your change will be Series[complex]. The reason is that the first overload doesn't get matched because the type of the Series is Series[S1], and the first overload requires the scalar to be of the same type S1. So with your change, you get Series[complex], because it now matches the second overload, because the float value of 1.2 gets matched to the complex argument.

To get this to work, you have to consider ALL the possible combinations, and get the overloads in the right order. This is non-trivial.

@shirzady1934
Copy link
Contributor Author

I changed it and placed all in right order but typechecking for pyright failed

code:

eries[int], other: int) -> Series[int]: ...
    @overload
    def __add__(self: Series[int], other: float) -> Series[float]: ...
    @overload
    def __add__(self: Series[int], other: complex) -> Series[complex]: ...
    @overload
    def __add__(self: Series[float], other: int) -> Series[float]: ...
    @overload
    def __add__(self: Series[float], other: float) -> Series[float]: ...
    @overload
    def __add__(self: Series[float], other: complex) -> Series[complex]: ...
    @overload
    def __add__(self, other: S1 | Self) -> Self: ...
    @overload
    def __add__(
        self,
        other: num | _str | timedelta | Timedelta | _ListLike | Series | np.timedelta64,
    ) -> Series: ...
    # ignore needed for mypy as we want different results based on the arguments
    @overload  # type: ignore[override]
    def __and__(  # pyright: ignore[reportOverlappingOverload]
        self, other: bool | list[int] | MaskType
    ) -> Series[bool]: ...
    @overload
    def __and__(  # pyright: ignore[reportIncompatibleMethodOverride]
        self, other: int | np_ndarray_anyint | Series[int]
    ) -> Series[int]: ...

logs:

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

Success: no issues found in 226 source files

===========================================
End: 'Run mypy on 'tests' (using the local stubs) and on the local stubs', runtime: 31.722 seconds.
===========================================


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

/home/kicker_angel/contr/pandas-stubs/pandas-stubs/core/series.pyi
  /home/kicker_angel/contr/pandas-stubs/pandas-stubs/core/series.pyi:1580:9 - error: Overload 1 for "__add__" overlaps overload 2 and returns an incompatible type (reportOverlappingOverload)
  /home/kicker_angel/contr/pandas-stubs/pandas-stubs/core/series.pyi:1580:9 - error: Overload 1 for "__add__" overlaps overload 3 and returns an incompatible type (reportOverlappingOverload)
  /home/kicker_angel/contr/pandas-stubs/pandas-stubs/core/series.pyi:1586:9 - error: Overload 4 for "__add__" overlaps overload 6 and returns an incompatible type (reportOverlappingOverload)
3 errors, 0 warnings, 0 informations 

===========================================
Step: 'Run pyright on 'tests' (using the local stubs) and on the local stubs' failed!
===========================================

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Mar 16, 2025

/home/kicker_angel/contr/pandas-stubs/pandas-stubs/core/series.pyi:1580:9 - error: Overload 1 for "add" overlaps overload 2 and returns an incompatible type (reportOverlappingOverload)
/home/kicker_angel/contr/pandas-stubs/pandas-stubs/core/series.pyi:1580:9 - error: Overload 1 for "add" overlaps overload 3 and returns an incompatible type (reportOverlappingOverload)
/home/kicker_angel/contr/pandas-stubs/pandas-stubs/core/series.pyi:1586:9 - error: Overload 4 for "add" overlaps overload 6 and returns an incompatible type (reportOverlappingOverload)

It's OK to add # pyright: ignore[reportOverlappingOverload] on the offending lines in the stubs. We do that elsewhere.

You should look at the PR #1093 to see what was attempted.

@loicdiridollou can say more. This is VERY hard to get right.

@shirzady1934 shirzady1934 marked this pull request as draft March 16, 2025 21:28
@loicdiridollou
Copy link
Contributor

Indeed, the big trick here is how to manage the UnknownSeries. It was introduced as a super type of Series[Any] recently and it may help unlock some of the possibilities.
When I started building the PR this is the very issue that I faced. The other trick is that you have to do all or nothing which creates quite a big PR, no chance of walking one step after the other and making one small PR at a time.
Now that UnknownSeries was introduced you may have a much better shot at wrapping this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants