quantopian / empyrical

Common financial risk and performance metrics. Used by zipline and pyfolio.
https://quantopian.github.io/empyrical
Apache License 2.0
1.27k stars 398 forks source link

backwards compat: empyrical.cum_returns_final used to support dataframes #100

Closed twiecki closed 6 years ago

twiecki commented 6 years ago

https://github.com/quantopian/bayesalpha/commit/c57713fb7db2fd5e10804d1e1e74ee543ddd3dbd broke a (non-explicit and apparently untested) use-case where a user can pass in a DataFrame with multiple columns and get the cumulative returns for each column. After this change, it always returns a scalar. This quietly breaks user-code.

Ideally we revert it to the old way, or at least raise an error that dataframe inputs are not supported.

CC @llllllllll

ssanderson commented 6 years ago

@twiecki cum_returns appears to work fine for me on master:

In [16]: data = pd.DataFrame({'a': np.full(10, 0.1), 'b': np.full(10, 0.15)})

In [17]: data
Out[17]:
     a     b
0  0.1  0.15
1  0.1  0.15
2  0.1  0.15
3  0.1  0.15
4  0.1  0.15
5  0.1  0.15
6  0.1  0.15
7  0.1  0.15
8  0.1  0.15
9  0.1  0.15

In [18]: cum_returns(data)
Out[18]:
          a         b
0  0.100000  0.150000
1  0.210000  0.322500
2  0.331000  0.520875
3  0.464100  0.749006
4  0.610510  1.011357
5  0.771561  1.313061
6  0.948717  1.660020
7  1.143589  2.059023
8  1.357948  2.517876
9  1.593742  3.045558

Is the issue here with cum_returns_final? That does indeed appear to return a scalar for me:

In [20]: cum_returns_final(data)
Out[20]: 9.493134873891622
twiecki commented 6 years ago

Is the issue here with cum_returns_final? That does indeed appear to return a scalar for me:

Yes, indeed.