calculation issues with rolling_sharpe() and rolling_sortino()

[kartiksubbarao]

I noticed a couple of calculation issues with the rolling_sharpe() and rolling_sortino() functions used in the tearsheet graphs:

1. In both rolling_sharpe() and rolling_sortino(): The period variable is used to specify the number of days in the rolling window (by default 126). However, period is also used to annualize the daily returns data for returns and benchmarks, which is incorrect. Irrespective of the number of days in the rolling window, the returns are still daily returns, so the factor should be 252. [For Creating reports on monthly returns data #74, a new variable would need to be created -- period can't do double duty as both the number of datapoints in the rolling window, AND the timespan that each datapoint represents :-) ]
2. In rolling_sortino() in wrappers.py, the target downside deviation is not calculated as per the Red Rock Capital whitepaper cited in the sortino() function in stats.py. rolling_sortino() throws away all the positive returns and only takes the standard deviation of the negative returns, which is the opposite of the whitepaper's recommendation. I'm currently using this in rolling_sortino() as a workaround:

   from empyrical import roll_sortino_ratio
   returns = roll_sortino_ratio(returns, period)
   

I'd be happy to provide a pull request for these items, just let me know.

[ranaroussi]

Item no. 1 was fixed in 0.0.34.

As for item 2 - if you can provide a PR that would be great :)

[kartiksubbarao]

I did a benchmark with a pandas rolling apply() implementation compared to using empyrical's roll_sortino_ratio(). The empyrical function was over 50x faster, so I went with that :-) I realize it does introduce a dependency to empyrical, but I've seen that empyrical's functions are pretty efficient, you might find it useful to leverage in other places as well. Obviously your call of course. Thanks again for all of your work on quantstats!

[kartiksubbarao]

Here is the pull request: #100

[ranaroussi]

Hi Kartik,

Mind posting Pandas rolling apply() implementation here? I'll try to optimize it as I'm reluctant to add another dependency.

Thanks!

[kartiksubbarao]

Here you go: kartiksubbarao@853da02
There's just one obvious optimization to skip unnecessary calls to _prepare_returns(). This is what I benchmarked as 50x+ slower than empyrical's roll_sortino_ratio().

[kartiksubbarao]

@ranaroussi 7156af7 fixes some of these issues, but there is still a calculation issue with rolling_sortino(). I'll close this and report a new issue for that one.