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

[phofl]

• closes BUG: df.__setitem__ can be 10x slower than pd.concat(..., axis=1) #37954
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• 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

[jreback]

wow, simplification and pref boost

[jreback]

can you create the keys in the same order whether they are in the obj or not (e.g. combine L666 and 667)

[jreback]

do we have a test for this ordering?

[phofl]

Union should do the trick.

At least the test added with #37964 covers this

[jreback]

lgtm can merge on green.

[phofl]

@jreback greenish. Failure unrelated

[jreback]

thanks @phofl

[jbrockmendel]

@phofl did you determine that this isn't making a copy?

[phofl]

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?

[jbrockmendel]

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

[jreback]

i am not sure this is possible w/o a copy

[phofl]

This does make a copy. Did not know, that we have to compare values instead of the object itself.

[phofl]

Actually this raised previously too, so at least not a regression

[jbrockmendel]

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

[simonjayhawkins]

@meeseeksdev backport 1.1.x

[simonjayhawkins]

#38181 (comment)

@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

[jorisvandenbossche]

As @jbrockmendel noted, this can change the copy/view semantics in certain cases.

One example:

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)

[jbrockmendel]

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