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

Trying to make pd.concat work with pyright strict mode. #955

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

JanEricNitschke
Copy link
Contributor

Steps are:

  • Add explicit anys
  • Add tests that should work already
  • Add explicit S1 overloads

The first step is done. The second step does not work because of mypy. Those test failures also happen without the explicit Any.

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Jul 1, 2024

The first step is done. The second step does not work because of mypy. Those test failures also happen without the explicit Any.

I made a copy of your PR, and played around with it. I think this is a bug in mypy for which there is no easy workaround.

First, one change I did is to change the last overload of __new__() in series.pyi to return Series[Any] rather than Series. That will make mypy recognize that series2 declared at https://github.com/JanEricNitschke/pandas-stubs/blob/5a53dfca1cfb04263d136f5f2fda3cafcfa15b5d/tests/test_pandas.py#L59

to be type Union[list[None | pd.Series[Any]]. Then if I remove the last overload for pd.concat(), mypy will get past the first test that uses series2. But removing that overload causes other tests to fail.

So the issue here is that we have to return different results dependent on whether DataFrame is present in the Iterable passed to concat. So we need that last overload, but when that overload is present, then mypy isn't matching the test pd.concat([None, series2]) to the correct overload.

This is one of the challenges with our stubs - making them work with pyright and mypy, when the two type checkers have different behaviors in certain cases, and, in some cases, it is due to mypy bugs.

@JanEricNitschke
Copy link
Contributor Author

Thanks for checking this out. Then i guess the best we can do here is just taking the first commit for now and keeping it at that until the behaviour between mypy and pyright is consistent or at least can be made compatible here.

@JanEricNitschke
Copy link
Contributor Author

I removed the second commit that added the tests and just kept it add the basic addition of the Any type parameters.

@JanEricNitschke JanEricNitschke marked this pull request as ready for review July 8, 2024 09:26
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.

@Dr-Irv Dr-Irv merged commit e78aaca into pandas-dev:main Jul 8, 2024
26 checks passed
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.

2 participants