-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: Fix replace for different dtype equal value #34878
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 13 commits
47c83f9
f45017f
268056c
1e01477
60ef5fc
d7f1fa8
bd46f2d
c6eb99b
d87954e
93d7186
3430201
a428f64
b747b6d
464c81d
5bb9b71
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 |
---|---|---|
|
@@ -714,9 +714,13 @@ def replace( | |
# retry | ||
if not self._can_hold_element(to_replace): | ||
if not isinstance(to_replace, list): | ||
if inplace: | ||
return [self] | ||
return [self.copy()] | ||
return self.astype(object).replace( | ||
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. we only want to do this if these are different types, not generally. please show asv's for this. 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 the Not seeing any performance regressions from the change (at least in the replace benchmarks):
|
||
to_replace=to_replace, | ||
value=value, | ||
inplace=inplace, | ||
regex=regex, | ||
convert=convert, | ||
) | ||
|
||
to_replace = [x for x in to_replace if self._can_hold_element(x)] | ||
if not len(to_replace): | ||
|
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.
can you move to 1.2
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.
this looks similar to regression reported in #35376 with open PR (with different fix) #36444
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.
Yeah I think it's the same issue. If another fix avoids astyping to object I would think it's preferred.