Skip to content

BUG: add back check for MultiIndex case and take_split_path #27855

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

Merged
merged 4 commits into from
Aug 13, 2019

Conversation

jorisvandenbossche
Copy link
Member

Closes #27841

@jorisvandenbossche
Copy link
Member Author

This is reverting part of #27351: there was a seemingly unused check removed (as we thought it was for panels), but apparently it was still used (given the regression report in #27841)

There was actually discussion about this here: https://github.com/pandas-dev/pandas/pull/10838/files#r302728324

@jorisvandenbossche
Copy link
Member Author

Updated.

I could simplify the case, and understand now better when this was triggered: it's when adding a single column, but where the single column selection is a slice.

In a non-MultiIndex case we actually don't allow this (and you get the same error as now in 0.25 with MultiIndex):


In [61]: df
Out[61]: 
          a         b         c
0 -2.791318 -0.238227  0.791717
1  0.635844 -1.494229  0.445973
2  0.575157  0.295088 -0.978239
3  0.894169  0.300125 -0.949338
4  0.716657 -0.634217 -2.311306

In [62]: df.loc[:, 'a':'a']
Out[62]: 
          a
0 -2.791318
1  0.635844
2  0.575157
3  0.894169
4  0.716657

In [63]: df.loc[:, 'a':'a'] = np.arange(5) 
...
ValueError: could not broadcast input array from shape (5) into shape (5,1)

But for the MultiIndex, you can get a "slice" without that the user explicitly slices the columns (as I did for the non-MultiIndex case above). So there we are more flexible.
We could maybe discuss if we want to be more strict here as well (in the future, on purpose with a deprecation), but those MultiIndex corner cases are all a bit tricky.

@jbrockmendel
Copy link
Member

I could simplify the case, and understand now better when this was triggered: it's when adding a single column, but where the single column selection is a slice.

Simplification sounds good. If there's a reasonable scope to refactor into its own method, that would be worthwhile; the setitem_with_indexer method is a beast.

@TomAugspurger
Copy link
Contributor

Merged master.

I think this is OK to fix the regression. I doubt we would want to backport any simplification code anyway.

@jbrockmendel jbrockmendel merged commit aef0804 into pandas-dev:master Aug 13, 2019
@jbrockmendel
Copy link
Member

Thanks @jorisvandenbossche

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Aug 13, 2019
@jorisvandenbossche jorisvandenbossche deleted the GH27841 branch August 14, 2019 07:08
quintusdias pushed a commit to quintusdias/pandas_dev that referenced this pull request Aug 16, 2019
…ev#27855)

* BUG: add back check for MultiIndex case and take_split_path

* update comment

* add simpler test + whatsnew
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Regression Functionality that used to work in a prior pandas version
Projects
None yet
3 participants