-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
CLN: enforce deprecation of interpolate
with object dtype
#57820
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
Changes from 6 commits
fb51b69
b2ee587
3ae7d75
cb2179f
ea76776
823e467
e3c56a8
05f805c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7845,16 +7845,21 @@ def interpolate( | |
obj, should_transpose = self, False | ||
else: | ||
obj, should_transpose = (self.T, True) if axis == 1 else (self, False) | ||
if np.any(obj.dtypes == object): | ||
# GH#53631 | ||
if not (obj.ndim == 2 and np.all(obj.dtypes == object)): | ||
# don't warn in cases that already raise | ||
warnings.warn( | ||
f"{type(self).__name__}.interpolate with object dtype is " | ||
"deprecated and will raise in a future version. Call " | ||
"obj.infer_objects(copy=False) before interpolating instead.", | ||
FutureWarning, | ||
stacklevel=find_stack_level(), | ||
# GH#53631 | ||
if obj.ndim == 1 and obj.dtype == object: | ||
raise TypeError( | ||
f"{type(self).__name__} cannot interpolate with object dtype." | ||
) | ||
if obj.ndim == 2: | ||
if np.all(obj.dtypes == object): | ||
raise TypeError( | ||
"Cannot interpolate with all object-dtype columns " | ||
"in the DataFrame. Try setting at least one " | ||
"column to a numeric dtype." | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can now simplify things and remove this check. The reason: we raise
in case
? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch! Yeah try that out There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But the messaging will need to probably change, this could be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry, I don't understand why is it a I suggest to use the message
I think we can just remove the block with the old message:
and change the old message in tests to the new one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah sorry I had misunderstood your initial suggestion. You can also go a step further and replace All of these checks with if np.any(obj.dtypes == object):
raise TypeError(f"{type(self).__name__} cannot interpolate with object dtype.") Since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry, my wording wasn't clear. My idea was to remove the second part of the error message:
I made changes. Could you please take a look at it? |
||
elif np.any(obj.dtypes == object): | ||
raise TypeError( | ||
f"{type(self).__name__} cannot interpolate with object dtype." | ||
) | ||
|
||
if method in fillna_methods and "fill_value" in kwargs: | ||
|
@@ -7871,13 +7876,6 @@ def interpolate( | |
|
||
limit_direction = missing.infer_limit_direction(limit_direction, method) | ||
|
||
if obj.ndim == 2 and np.all(obj.dtypes == object): | ||
raise TypeError( | ||
"Cannot interpolate with all object-dtype columns " | ||
"in the DataFrame. Try setting at least one " | ||
"column to a numeric dtype." | ||
) | ||
|
||
if method.lower() in fillna_methods: | ||
# TODO(3.0): remove this case | ||
# TODO: warn/raise on limit_direction or kwargs which are ignored? | ||
|
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'd suggest doing this check inside Block.interpolate
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.
there's a TODO(3.0) comment related to this inside Block.interpolate
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.
thank you, I found the comment. In #58083 I moved raising TypeError from
NDFrame
toBlock
.I think maybe it's better to change the error message from
"NumpyBlock cannot interpolate with object dtype."
to"Can not interpolate with object dtype."
?