-
-
Notifications
You must be signed in to change notification settings - Fork 132
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
Fix signature of Series.map() #942
Conversation
You could fix that by doing this in
We haven't tested the stubs yet with numpy 2.0, and I think it might be a bit of work for us to get that to work with both the old and new numpy versions. |
I have now added two tests for this that fail without the changes and pass with them. I have done the pyproject.toml changes locally for this. There was still one test failure for I wouldnt put the pyproject.toml into this PR because i feel i wouldnt want to restrict my runtime dependencies just for the stubs, especially since they may still correctly type hint my use case (which for me personally they do as far as i know) |
So the CI is failing because it installed numpy 2.0. I think I have a fix for that. I am going to try that out and if it works, get that merged in, then you can merge in that change, and I think your CI will then succeed. |
@JanEricNitschke we just merged in the changes so the CI will work with numpy 2.0. Can you fetch main, merge it into your branch, and push? |
d263eea
to
2c9464d
Compare
Rebased. Locally i still have the Also: Do you accept PRs for things that only affect pyright strict mode? In that case the added tests might not fail without the changes with the current pyright settings of pandas-stubs. (Dont have such an example yet, but might in the future as i am trying to get a project that uses pandas to type check in strict mode) |
We know that test is failing because of So I believe if you removed There's a possibility that
As long as any fix that is good for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @JanEricNitschke
The output of show_versions is:
The locally failing test gives:
Great, just wanted to make sure because it said somewhere that there should always be a test that fails without and works with. But i did realize that that would be a lot of work to setup. |
@JanEricNitschke I think the issue here is that you don't have If you had followed these instructions: https://github.com/pandas-dev/pandas-stubs/blob/main/docs/setup.md Then I think |
@Dr-Irv i actually did follow those steps. Except i had already installed poetry via pipx. I guess that might cause the difference? |
@JanEricNitschke Not sure. The error is definitely because |
Series.map()
needs type forarg
and result #941assert_type()
to assert the type of any return valueOnly added the proposed changes so far. Will try to work on tests as soon as possible.
Does not seem to break any existing tests so far at least.
(I think the failures are all due to numpy 2.0 and separate from my changes)