MAINT: code refactoring to make it compatible with pandas 0.23.4 #328
Conversation
| @@ -724,6 +724,11 @@ def plot_cumulative_returns(factor_returns, period, title=None, ax=None): | |||
| Length of period for which the returns are computed (e.g. 1 day) | |||
| if 'period' is a string it must follow pandas.Timedelta constructor | |||
| format (e.g. '1 days', '1D', '30m', '3h', '1D1h', etc) | |||
| freq : pandas DateOffset | |||
| Used to specify a particular trading calendar e.g. BusinessDay or Day | |||
| Usually this is inferred from utils.infer_trading_calendar, which is | |||
There was a problem hiding this comment.
If it's inferred, why is it a required argument?
There was a problem hiding this comment.
Because plot_cumulative_returns doesn't receive factor_data in input, which has the inferred trading calendar.
|
wasn't there a commit to add newer pandas to travis? |
Yes, but I removed it because there are changes in pandas 0.23.4 that conflict with pandas 0.18.1 and some tests fail. Those tests are not that meaningful, e.g. some nans are returned as 0s in new pandas and some indices need to be Categorical. Anyway I didn't want to chase what changed internally in pandas, so when we get rid of pandas 0.18.1 we can update the tests for new pandas versions. |
|
Thanks @luca-s! |
Added
freqargument toplot.plot_cumulative_returnsandplot.plot_cumulative_returns_by_quantile, which allow to avoid settings the index frequency insideperf.to_weightsandperf.factor_returns(which in turn causes pandas 0.23.4 to crash, see #319 )Also this closes #319