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.

Sign up for GitHub

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

Jump to bottom

Fix regression in iloc with boolean list #37432

Merged
merged 8 commits into from
Oct 28, 2020
Merged

Fix regression in iloc with boolean list #37432

merged 8 commits into from
Oct 28, 2020

Conversation

phofl
Copy link
Member

@phofl phofl commented Oct 26, 2020

Run tests in indexes and indexing locally without failure.

@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 Oct 26, 2020
@phofl phofl added this to the 1.1.4 milestone Oct 26, 2020
@phofl
Copy link
Member Author

phofl commented Oct 26, 2020

cc @simonjayhawkins @jreback

Is this early enough to get into 1.1.4?

simonjayhawkins
simonjayhawkins reviewed Oct 27, 2020
View reviewed changes
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @phofl for the PR. I won't comment on the fix and leave that to @jreback and @jbrockmendel

pandas/tests/indexing/test_iloc.py Outdated
@@ -767,3 +767,11 @@ def test_iloc_setitem_series_duplicate_columns(self):
)
df.iloc[:, 0] = df.iloc[:, 0].astype(np.float64)
assert df.dtypes.iloc[2] == np.int64


def test_iloc_setitem_bool_array():
Copy link
Member

Choose a reason for hiding this comment

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

could also be put somewhere in pandas\tests\frame\indexing\

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that's the file I looked for there :) Thanks

@simonjayhawkins simonjayhawkins mentioned this pull request Oct 27, 2020
Closed
5 tasks
@phofl
Move test
057130c
@phofl
Merge branch 'master' of https://github.com/pandas-dev/pandas into 36741
3a5367c 
� Conflicts:
�	pandas/tests/frame/indexing/test_setitem.py
@simonjayhawkins
Copy link
Member

@phofl it appears the test got lost in the merge

@phofl
Copy link
Member Author

phofl commented Oct 27, 2020

Thanks very much. That was not that clever :)

@jreback
Copy link
Contributor

jreback commented Oct 27, 2020

code fix is ok, but this is going to be tricky to backport as this code as changed a bit.

cc @jbrockmendel merge if looks good to you.

jreback
jreback reviewed Oct 27, 2020
View reviewed changes
pandas/tests/frame/indexing/test_setitem.py Outdated
@@ -229,3 +229,10 @@ def test_setitem_periodindex(self):
rs = df.reset_index().set_index("index")
assert isinstance(rs.index, PeriodIndex)
tm.assert_index_equal(rs.index, rng)

def test_iloc_setitem_bool_array(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

this is actually a list can you parameterize on
[list, np.array] e.g.

indexer = klass([True, False, False])
df.iloc[indexer, 1] = .....

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Done. Renamed test too.

@phofl @jorisvandenbossche
Update doc/source/whatsnew/v1.1.4.rst
63d34d4 
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@jreback jreback merged commit 5292533 into pandas-dev:master Oct 28, 2020
@jreback
Copy link
Contributor

jreback commented Oct 28, 2020

this is fine, thanks @phofl

not sure how hard the actual backport to 1.1.4 is

@jreback
Copy link
Contributor

jreback commented Oct 28, 2020

@meeseeksdev backport 1.1.x

@lumberbot-app
Copy link

lumberbot-app bot commented Oct 28, 2020

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout 1.1.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 52925335fe6d5f1ea6c79bb03da70654d9be9668
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #37432: Fix regression in iloc with boolean list'
  1. Push to a named branch :
git push YOURFORK 1.1.x:auto-backport-of-pr-37432-on-1.1.x
  1. Create a PR against branch 1.1.x, I would have named this PR:

"Backport PR #37432 on branch 1.1.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

@jreback
Copy link
Contributor

jreback commented Oct 28, 2020

yep this needs manual. @phofl @simonjayhawkins

@phofl phofl deleted the 36741 branch October 28, 2020 08:09
phofl added a commit to phofl/pandas that referenced this pull request Oct 28, 2020
@phofl
Fix regression in iloc with boolean list (pandas-dev#37432)
fdf14f4 
(cherry picked from commit 5292533)
simonjayhawkins pushed a commit that referenced this pull request Oct 28, 2020
@phofl
Fix regression in iloc with boolean list (#37432) (#37463)
91b7aba 
(cherry picked from commit 5292533)
kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
@phofl
Fix regression in iloc with boolean list (pandas-dev#37432)
abaa2eb
ukarroum pushed a commit to ukarroum/pandas that referenced this pull request Nov 2, 2020
@phofl @ukarroum
Fix regression in iloc with boolean list (pandas-dev#37432)
04b9561
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonjayhawkins simonjayhawkins simonjayhawkins left review comments

@jorisvandenbossche jorisvandenbossche jorisvandenbossche left review comments

@jreback jreback jreback left review comments

Assignees
No one assigned
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
Milestone
1.1.4
Development

Successfully merging this pull request may close these issues.

BUG: Setting values with ILOC and list like indexers raises
4 participants
@phofl @simonjayhawkins @jreback @jorisvandenbossche