PERF: faster plotting of timeseries with PeriodIndex

[jorisvandenbossche]

See discussion in #4705.

This improves the speed of plotting timeseries with PeriodIndex (or with DatetimeIndex that is converted to a PeriodIndex during plotting) drastically: from 30 s in 0.12 (but 7 s in 0.11) to 0.2 s with this commit for the plotting of following data:

N = 10000
M = 25
df = pd.DataFrame(np.random.randn(N,M), index=pd.date_range('1/1/1975', periods=N))
df.plot()

Some questions I still have:

• I added a check for PeriodIndex, but I left the original if isinstance(values, Index) in place. Is this needed? Is it possible that another index than PeriodIndex ends up in PeriodConverter?
• @jreback You asked in the issue to do some tests (to at least ensure that their is a valid plot). Do you just mean by plotting it and looking if it seems OK? This seems OK. Or can you do some tests for plotting in the testing infrastructure?
• Would it be interesting to add a performance test for this case? Because it was a big decrease in speed with 0.12 which was not detected.

[jreback]

add the tests here

https://github.com/pydata/pandas/blob/master/pandas/tests/test_graphics.py

[jreback]

@jorisvandenbossche I think you could try to create a vbench for this, nothing there now but create a file tlike this: (named say plotting or graphics): https://github.com/pydata/pandas/blob/master/vb_suite/ctors.py, and add it to the suite.py.

test using:

test_perf.sh -b <commit prior> -t <your last commit>

(lots of options to test_perf, e.g. you might want to start all of the tests with plot_, then you can add the
-r plot optino to ONLY test thoses specific vbenches

[jtratner]

@jorisvandenbossche matplotlib actually tests this by comparing images. I don't know whether it's usable in client packages though.

[jorisvandenbossche]

@jreback and @jtratner Thanks for the comments. Adding tests with comparing images does lead a little bit too far for this PR (and for me) I think (that is another discussion), but I will look at adding it to the current tests (the _ckeck_plot_works) and to add something to the vbench suite.

What is actually the difference between pandas/bench and pandas/vb_suite?

[jtratner]

vb_suite tests pandas performance against itself - used for testing that changes don't introduce performance regressions. bench - pandas vs. other software.

On testing images - that seems fine to me, don't know what @jreback thinks.

[jorisvandenbossche]

I added something to the vbench suite (see commit). This is the output I get when I run it with test_perf.sh ... -r plot:

-------------------------------------------------------------------------------
Test name                                    | head[ms] | base[ms] |  ratio   |
-------------------------------------------------------------------------------
plot_timeseries_period                       | 267.8120 | 21528.0979 |   0.0124 |
-------------------------------------------------------------------------------

Ratio < 1.0 means the target commit is faster then the baseline.
Seed used: 1234

Target [7c4866b] : PERF: add plotting to vbench
Base   [c472099] : Merge pull request #4613 from jtratner/refactor-datetimes

The base is now taking 21 s to run (the target 0.26 s), should I decrease the size of the dataframe so it runs faster?

[jtratner]

If it's too fast (i.e., too small of a dataframe), it makes it harder to tell if there are actually differences...@jreback? @y-p?

[jreback]

prob make a bit smaller so that the older/slower one doesn't take so long
you will still see a similar ratio

[jorisvandenbossche]

OK, I added a test (actually there was already a PeriodSeries defined, but not used in a test) and changed the dimensions of the dataframe in the benchmark (it now runs in 0.9 s for base and 0.07 s for target)

Should something be added to release notes? (it is not a bug fix, so maybe not?)

[hayd]

Sure it's a bug-fix (it fixes 4705) so you should add to release.rst, also mention you've added it to vbench. :)

[jorisvandenbossche]

Oeps, git-newbie here. I wanted to finish this up, but while rebasing interactive to squash some commits I also picked the last three commits from @jreback

How can I remove them from this PR? (or is it not a problem for the history that it is included here in the overview?)

[jreback]

@jorisvandenbossche

rebase again to 1 commit prior to yours and force push

[jorisvandenbossche]

@jreback What do you mean exactly? I dit git rebase -i HEAD~2 and a forced push, but your commits are still here.

[jreback]

I think this might work git rebase -i origin/master

[cpcloud]

@jorisvandenbossche @jreback's cmd will only work if your origin/master is up to date with upstream/master....

better to do

git remote add upstream git://github.com/pydata/pandas.git
git fetch upstream
git rebase upstream/master

[jreback]

@cpcloud just about to ask! you have git-fu.

I think I solved this once by

git checkout master
git pull 
git checkout -b new_branch --track master
git cherry_pick old_branch_commits (one by one)

but a little kludgy

[jorisvandenbossche]

OK, I also just did a simple git rebase upstream/master, and this indeed seems to fixed it!

[cpcloud]

great!

[cpcloud]

@jorisvandenbossche you might want to read this wiki section. it gives a brief overview of rebase and merge

[jorisvandenbossche]

@cpcloud Yeah, the basic rebase and merge I (think I) understand now, but it was because I picked too many commits with a rebase -i HEAD~4 to squash some commits, that I 'recommited' some commits from jreback ..

I don't know there is something else that I have to do for this PR, but for me this is ready to merge.

[cpcloud]

also @jorisvandenbossche if you ever run into this problem again one way to resolve it is this:

let's say you're 2 commits ahead of upstream/master

git fetch upstream  # don't forget this otherwise the next line will put you in a different state than current master!
git reset HEAD~2
git stash  # push on to the stash stack
git reset --hard upstream/master  # get back on master
git stash pop  # pop off the stack
git add .
git commit -m'my aw-sum commit mess-ige'

[cpcloud]

@jreback how's that for kludgy? 😄

[jorisvandenbossche]

I think I prefer the git fetch upstream, git rebase upstream/master then :-)

[jreback]

@cpcloud and I thought mine was kludgy!

[jorisvandenbossche]

But back tot the PR, anything else I have to do? For me it is ready to merge.

[cpcloud]

looks okay on my end

[jreback]

@jorisvandenbossche thanks much!

[jorisvandenbossche]

@jreback Enjoying that I can help!

[jtratner]

or just use git reflog and go back to the last time the branch was good :)