Skip to content

Conversation

loicdiridollou
Copy link
Member

@loicdiridollou loicdiridollou commented Apr 29, 2025

Turns out there is a nasty edge case when you pass everything as None for upper and lower and ask for inplace=True, it will return Self and not None.

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.

can you add tests where either lower or upper is not specified?

I think that might lead to some changes in how you wrote the overloads.

@loicdiridollou
Copy link
Member Author

Turns out no changes were needed but agreed that this time we check all the cases.

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Apr 30, 2025

I'm a little confused here, as you closed the PR and then reopened it. Should I review?

@loicdiridollou
Copy link
Member Author

loicdiridollou commented Apr 30, 2025 via email

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.

I think you may also need similar overloads for clip in series.pyi and then similar tests that omit the lower or upper arguments.

@loicdiridollou
Copy link
Member Author

loicdiridollou commented Apr 30, 2025 via email

@loicdiridollou
Copy link
Member Author

loicdiridollou commented May 1, 2025

Turns out the axis parameter does not need anything specific (to split the overloads) since it is not used (unless you pass something not allowed).
So the overload did not need anything additional yet I have added more tests to lock in the behavior.

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 fc331b1 into pandas-dev:main May 1, 2025
13 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.

Create overloads for DataFrame.clip()

2 participants