-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: make Index.where behavior mirror Index.putmask behavior #39412
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
jbrockmendel
commented
Jan 26, 2021
- closes #xxxx
- tests added / passed
- Ensure all linting tests pass, see here for how to run them
- whatsnew entry
# coerces to object | ||
return self.astype(object).putmask(mask, value) | ||
dtype = self._find_common_type_compat(value) | ||
return self.astype(dtype).putmask(mask, value) |
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.
copy=False?
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.
we'll always be making a copy here since dtype != self.dtype
@@ -799,29 +799,22 @@ def length(self): | |||
return Index(self._data.length, copy=False) | |||
|
|||
def putmask(self, mask, value): | |||
arr = self._data.copy() | |||
mask = np.asarray(mask, dtype=bool) |
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't you share / use the array/interval putmask code here?
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.
we do; L814-815 is directly using IntervalArray.putmask. everything before that point is for ways that the Index method behaves differently from the array method
needs a rebase. can you add a whatsnew on what is changing here |
one more rebase |
rebased + green |
can you merge master |
rebased + finally green |