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

Simplify and fix overloads for methods with inplace parameter #1105

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

brianhelba
Copy link

The return value of these methods varies on the inplace argument.

The return value of these methods varies on the `inplace` argument.
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.

Tests are failing. Please make sure to run the tests locally before resubmitting.

I understand what you are trying to do here, and it seems correct, but I also would like to know what prompted you to make these changes. Is there a test we can add that fails with the current stubs but passes with your changes?

Comment on lines -514 to -519
level: Sequence[Level] = ...,
*,
drop: bool = ...,
name: Level = ...,
inplace: Literal[True],
allow_duplicates: bool = ...,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you can remove this overload.

level: Level | None = ...,
errors: IgnoreRaise = ...,
) -> Series | None: ...
) -> Series[S1]: ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be Self

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