ENH: Avoid copying whole block for single block case #51435
Conversation
| # Split blocks to only copy the columns we want to modify, only 1 block | ||
| if self.ndim == 2 and isinstance(indexer, tuple): | ||
| blk_loc = self.blklocs[indexer[1]] | ||
| if is_list_like(blk_loc) and blk_loc.ndim == 2: |
There was a problem hiding this comment.
how would you get blk_loc.ndim == 2?
There was a problem hiding this comment.
Would have to check, got test failures without checking this
There was a problem hiding this comment.
Did you check this again?
There was a problem hiding this comment.
Forget to comment maybe_convert_indexer makes indexer[1] 2D in some cases which causes blk_loc.ndim=2
There was a problem hiding this comment.
What indexer[1] causes this? this strikes me as very weird
There was a problem hiding this comment.
test_loc_setitem_reordering_with_all_true_indexer
That's one of the test that hits this if you want to take a look
There was a problem hiding this comment.
i added this check and a breakpoint on main and it didnt get hit in that test (or that test file). are there other changes in this PR that cause it to be reached?
pandas/core/internals/managers.py
Outdated
| elif not is_list_like(blk_loc): | ||
| # Keep dimension and copy data later | ||
| blk_loc = [blk_loc] | ||
| if len(blk_loc.shape) == 0: |
pandas/core/internals/managers.py
Outdated
| # Keep dimension and copy data later | ||
| blk_loc = [blk_loc] | ||
| if len(blk_loc.shape) == 0: | ||
| return self |
There was a problem hiding this comment.
IIRC we can only get here in setitem ops, so doesn't matter. Will check more closely though
| # Split blocks to only copy the columns we want to modify, only 1 block | ||
| if self.ndim == 2 and isinstance(indexer, tuple): | ||
| blk_loc = self.blklocs[indexer[1]] | ||
| if is_list_like(blk_loc) and blk_loc.ndim == 2: |
There was a problem hiding this comment.
Did you check this again?
| 0, blk_loc, values | ||
| ) | ||
| # first block equals values | ||
| self.blocks[0].setitem((indexer[0], np.arange(len(blk_loc))), value) |
There was a problem hiding this comment.
np.arange(len(blk_loc)) could also be a slice(None) ?
There was a problem hiding this comment.
It looks like this has side effects. Rolling back for now
Co-authored-by: Joris Van den Bossche <[email protected]>
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.let's see if this passes ci