-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: clip doesn't preserve dtype by column #24458
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
Conversation
looks pretty good - pls add. whatsnew note i suspect we have a couple of open issues for clip that might be closed by this as well if u can have a look |
pandas/core/generic.py
Outdated
subset = self.le(upper, axis=None) | isna(result) | ||
result = result.where(subset, upper, axis=None, inplace=inplace) | ||
if lower is not None: | ||
if inplace: |
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.
Why is this inplace
check only when lower is not None
? It looks like you also assign result = self
before the upper and lower check.
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.
Hi @mroeschke, thanks for reviewing!
result = self
is to ensure there is a return value in case both lower
and upper
are None, which they can. when inplace=True, result is None
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.
Got it. Thanks!
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.
I think its just better to use the existing idiom
if inplace:
self._update_inplace(result0
else:
return self
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.
actioned. thanks.
Codecov Report
@@ Coverage Diff @@
## master #24458 +/- ##
==========================================
- Coverage 92.29% 92.29% -0.01%
==========================================
Files 163 163
Lines 51969 51964 -5
==========================================
- Hits 47967 47962 -5
Misses 4002 4002
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #24458 +/- ##
==========================================
- Coverage 92.29% 92.29% -0.01%
==========================================
Files 163 163
Lines 51969 51943 -26
==========================================
- Hits 47967 47940 -27
- Misses 4002 4003 +1
Continue to review full report at Codecov.
|
had look at #20911, I think that's not a supported use case when passing a #21476 is to do with different precisions between s and threshold. It works fine downcasting from int to float but appears not from float32 to float64. I suspect it's to do with numpy and need more investigation. |
>>> df.clip(lower=2, upper=7)
floats int
0 2.0 2
1 2.0 2
2 2.2 2
3 3.3 3
4 4.4 4
5 5.5 5
6 6.6 6
7 7.0 7
8 7.0 7
9 7.0 7
>>> df.clip(lower=2, upper=7.5)
floats int
0 2.0 2.0
1 2.0 2.0
2 2.2 2.0
3 3.3 3.0
4 4.4 4.0
5 5.5 5.0
6 6.6 6.0
7 7.5 7.0
8 7.5 7.5
9 7.5 7.5 |
pandas/core/generic.py
Outdated
subset = self.le(upper, axis=None) | isna(result) | ||
result = result.where(subset, upper, axis=None, inplace=inplace) | ||
if lower is not None: | ||
if inplace: |
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.
I think its just better to use the existing idiom
if inplace:
self._update_inplace(result0
else:
return self
review actioned and added test and documentation. |
thanks. ping on green. |
@jreback thanks, it's green. |
thanks @minggli |
@jreback @mroeschke thanks for reviewing and happy new year! |
git diff upstream/master -u -- "*.py" | flake8 --diff
Before
After