ENH: Improve performance for df.__setitem__ with list-like indexers
State: closed

- [x] closes #37954
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Reindexing the Block Manager improves the performance significantly. I hope I have not missed anything, concerning the reindexing of the blocks. 
Time spent in ``_ensure_listlike_indexer`` is pretty low now.
timeit result for the ops methods:

```
In [20]: %timeit setitem(x, x_col, df)
1.09 ms ± 9.65 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

In [21]: %timeit concat(x, x_col, df)
293 µs ± 4.29 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
```

Should we add tests here? I have added an asv to capture this case.

cc @jbrockmendel 

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Performance
description: Memory or execution speed performance

Comments:
lgtm can merge on green.
@jreback greenish. Failure unrelated
thanks @phofl 
@phofl did you determine that this isn't making a copy?

Not with a test, but I tested this in code.
```
x = self.obj
self.obj._mgr = self.obj._mgr.reindex_axis(keys, 0)
```

This also changes x, so that is not a copy, isn't it?

Should we add a test for this?
> This also changes x, so that is not a copy, isn't it?

Assuming homogeneous dtype for now, what im asking for is:

```
values = self.obj.values
self.obj._mgr = self.obj._mgr.reindex_axis(keys, 0)
new_values = self.obj.values

assert new_values is values   # <-- will raise if we made a copy
```
i am not sure this is possible w/o a copy

This does make a copy. Did not know, that we have to compare values instead of the object itself.
Actually this raised previously too, so at least not a regression
> Did not know, that we have to compare values instead of the object itself.

Note: the example above I assumed homogeneous dtypes.  More generally, we need to have `obj._mgr.blocks[n].values` be unchanged for each pre-existing n.

> i am not sure this is possible w/o a copy

A few things need to happen for it to be possible:

1) add `consolidate: bool = True` keyword to `BlockManager.reindex_axis` that then passes consolidate=consolidate to `reindex_indexer`
2) add `only_slice: bool = False` to `reindex_axis` and `reindex_indexer` so it can be passed to `_slice_take_blocks_ax0`
3) the `reindex_axis` call in this PR passes `consolidate=False, only_slice=True`
@meeseeksdev backport 1.1.x
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
```

2. Cherry pick the first parent branch of the this PR on top of the older branch:
```
$ git cherry-pick -m1 2f4110967c303c739abef67be63ba2206d64bbb9
```

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

```
$ git commit -am 'Backport PR #38148: ENH: Improve performance for df.__setitem__ with list-like indexers'
```

4. Push to a named branch :

```
git push YOURFORK 1.1.x:auto-backport-of-pr-38148-on-1.1.x
```

5. Create a PR against branch 1.1.x, I would have named this PR:

> "Backport PR #38148 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](https://github.com/MeeseeksBox/MeeseeksDev).
                
https://github.com/pandas-dev/pandas/pull/38181#issuecomment-735682557

> @phofl #38148 (comment) was this necessary? do we know what commit caused the performance regression. the existing code on 1.1.x looks much simpler to what was replaced on master.

If we don't backport this, we will need to move the release note on master
As @jbrockmendel noted, this *can* change the copy/view semantics in certain cases. 

One example:

```python
df = pd.DataFrame({'a': [1, 2, 3], 'b': [4, 4, 6]})
# get one column as a view of df
s = df['a']
# add columns with list-like indexer
df[['c', 'd']] = np.array([[.1, .2], [.3, .4], [.4, .5]])
# edit in place the first column to check view semantics
df.iloc[0, 0] = 100
```

on master this gives:
```
In [8]: df
Out[8]: 
     a  b    c    d
0  100  4  0.1  0.2
1    2  4  0.3  0.4
2    3  6  0.4  0.5

In [9]: s
Out[9]: 
0    1
1    2
2    3
Name: a, dtype: int64
```

where "a" and "b" columns were now copied in the `df[['c', 'd']] = ...` operation (shown by `s` not being updated). 

While on pandas 1.1.4, this copy didn't happen, and the series was actually updated:

```
In [6]: df
Out[6]: 
     a  b    c    d
0  100  4  0.1  0.2
1    2  4  0.3  0.4
2    3  6  0.4  0.5

In [7]: s
Out[7]: 
0    100
1      2
2      3
Name: a, dtype: int64
```

This of course closely relates to the recent discussions about improving the copy/view semantics (as currently those semantics are both not clear and largely untested).

(now the above is certainly a specific example to trigger the issue. In many cases we would also do a copy, eg if there would already have been float columns present in the example `df`)

what if instead of calling reindex_axis we called reindex_indexer and passed `indexer=slice(len(self.obj.columns))`?