Overlapping drawdowns

[jlopezpena]

The get_top_drawdowns function in timeseries.py sometimes returns overlapping drawdown periods, which should not happen. This code reproduces the issue:

import pyfolio as pf
stock_rets = pf.utils.get_symbol_rets('FB')
pf.timeseries.gen_drawdown_table(stock_rets)

which produces the following output:

Drawdown with index 6 starts on 2014-10-24, finishes on 2015-01-15
Drawdown with index 5 starts on 2014-10-28, which cannot be a peak as then it would have meant the end of the previous drawdown.

Problem occurs on timeseries.py lines 770-772:

            underwater = pd.concat(
                [underwater.loc[:peak].iloc[:-1],
                 underwater.loc[recovery:].iloc[1:]])

the actual peak is getting removed from the series, allowing a subsequent drawdown to engulf the one that just got cropped. The cropping should start on the index immediately after peak. Assuming that indexes are always day-based replacing line 771 by

[underwater.loc[:peak + pd.to_timedelta("1d")].iloc[:-1]

seems to fix the bug.

[jlopezpena]

Partially related to this, lines 770-772 seem like a rather inefficient way of removing data from the series, an overall more efficient way would be to directly drop the unneeded lines rather than chop and concat:

underwater.drop(underwater[peak: recovery].index[1:-1], inplace=True)

I can write a PR if this is a bug and not the intended behaviour.

[twiecki]

@jlopezpena Many thanks for reporting. A PR with a fix and your suggested improvement would be greatly appreciated.