PERF: fix long string representation

[ivanovmg]

• closes PERF: large perf regression in DataFrame repr #36636

• tests added / passed

• passes black pandas

• passes git diff upstream/master -u -- "*.py" | flake8 --diff

• whatsnew entry

• Fix long string representation for large dataframes.

• Eliminate for loop, which was filtering out the proper rows/columns to be displayed.

• Revert to the original implementation with concat-ing head+tail and left+right parts.

[WillAyd]

Seems more reasonable - how does the performance look?

[ivanovmg]

Before refactoring:

In [1]: import pandas as pd
In [2]: import numpy as np
In [3]: df = pd.DataFrame(np.random.randn(1_000_000, 10))
In [4]: %timeit repr(df)
19.8 ms ± 850 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)

After refactoring:

In [4]: %timeit repr(df)
2.36 s ± 47 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

After this fix:

In [4]: %timeit repr(df)
103 ms ± 707 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)

[jorisvandenbossche]

Thanks! Changes look good.

Would be nice to maybe further profile before refactor / after this PR to see where the additional slowdown is coming from (but doesn't necessarily need to be here)

[ivanovmg]

I guess, I see some area for improvement. In _truncate() we create a copy of dataframe to tr_frame. However, when making terminal output, this function is called once again, so the copy is created one more time.
If we move self.tr_frame = self.frame.copy() to init, then execution time will become close to 50 ms (twice as small as what this PR suggests right now).
However, if we use the original approach, when there was no copy created whatsoever (self.tr_frame = self.frame), then the same becomes 12 ms.

When refactoring I was concerned with the statement self.tr_frame = self.frame as the upcoming changes in tr_frame would have effect on the original dataframe. Therefore I decided to deal with the copy. If it is not an issue, then I will get rid of copy at all.

[jorisvandenbossche]

When refactoring I was concerned with the statement self.tr_frame = self.frame as the upcoming changes in tr_frame would have effect on the original dataframe. Therefore I decided to deal with the copy. If it is not an issue, then I will get rid of copy at all.

With "upcoming changes", you mean code that is now not yet in formats.py, but you are wanting to do in future PRs? (and if so, can you give some examples of what you have in mind?) Or is there now already some code that mutates tr_frame?

[ivanovmg]

By the "upcoming changes" I meant changes in tr_frame in the code, inside DataFrameFormatter.
Particularly, self.tr_frame = self.tr_frame.iloc[whatever] would change self.tr_frame. However if self.tr_frame and self.frame point to the same object, that would change self.frame as well, which is not what we expect when displaying the object. Or probably I do not fully understand the slicing via iloc.

Regarding the future RPs on the topic. Right now I am working on restructuring formatters, in an attempt to have them more aligned with each other. PR in progress #36510.

[jorisvandenbossche]

Thanks for the explanation!

self.tr_frame = self.tr_frame.iloc[whatever] would change self.tr_frame. However if self.tr_frame and self.frame point to the same object, that would change self.frame as well

Actually, with python assignment/reference semantics, the self.tr_frame.iloc[whatever] creates a new object and then assigning it to self.tr_frame = .. makes that the self.tr_frame attribute now refers to this new object, but it doesn't change the original object that self.tr_frame was pointing to. So basically the assignment only lets the self.tr_frame variable point to the new object without changing any object inplace.
So for this, we should normally not need to worry about mutation, and a copy should not be needed.

[jreback]

yeah i would remove the .copy as it is not necessary (you could also add a test to assert that we don't mutate the inpute), but doesn't need to be in this PR

[jorisvandenbossche]

@ivanovmg thanks for the quick follow-up!