Skip to content
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

Fix regression for setitem not aligning rhs with boolean indexer #39944

Merged
merged 5 commits into from Feb 22, 2021

Conversation

phofl
Copy link
Member

@phofl phofl commented Feb 20, 2021

reindexing solves the issue here before dispatching to iloc

@phofl phofl added Indexing Related to indexing on series/frames, not to indexes themselves Regression Functionality that used to work in a prior pandas version labels Feb 20, 2021
@@ -3263,6 +3263,8 @@ def _setitem_array(self, key, value):
key = check_bool_indexer(self.index, key)
indexer = key.nonzero()[0]
self._check_setitem_copy()
if isinstance(value, DataFrame):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does series hit this path?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I did this because value could be an array for example

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok great

@@ -566,3 +566,11 @@ def test_setitem_boolean_mask(self, mask_type, float_frame):
expected = df.copy()
expected.values[np.array(mask)] = np.nan
tm.assert_frame_equal(result, expected)

def test_setitem_boolean_mask_aligning(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you parameterize over .loc & setitem

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@jreback jreback added this to the 1.2.3 milestone Feb 21, 2021
@simonjayhawkins
Copy link
Member

simonjayhawkins commented Feb 22, 2021

reindexing solves the issue here before dispatching to iloc

maybe add words to that effect as a comment in the code

test failure unreleated. restarted job

@phofl
Copy link
Member Author

phofl commented Feb 22, 2021

Added something

@@ -3264,6 +3264,9 @@ def _setitem_array(self, key, value):
key = check_bool_indexer(self.index, key)
indexer = key.nonzero()[0]
self._check_setitem_copy()
if isinstance(value, DataFrame):
# GHä39931 reindex since iloc does not align
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise LGTM. Thanks @phofl

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx

@jreback jreback merged commit 88bcd2d into pandas-dev:master Feb 22, 2021
@jreback
Copy link
Contributor

jreback commented Feb 22, 2021

thanks @phofl

@jreback
Copy link
Contributor

jreback commented Feb 22, 2021

@meeseeksdev backport 1.2.x

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Feb 22, 2021
@phofl phofl deleted the 39931 branch February 22, 2021 23:06
simonjayhawkins pushed a commit that referenced this pull request Feb 23, 2021
…igning rhs with boolean indexer) (#39979)

Co-authored-by: patrick <61934744+phofl@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Indexing Related to indexing on series/frames, not to indexes themselves Regression Functionality that used to work in a prior pandas version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Index alignment behaviour
3 participants