New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Additional Calculations & Roll, Up, Down Helpers #47
Conversation
empyrical/stats.py
Outdated
@@ -587,7 +572,7 @@ def downside_risk(returns, required_return=0, period=DAILY, | |||
return dside_risk | |||
|
|||
|
|||
def information_ratio(returns, factor_returns): | |||
def excess_sharpe(returns, factor_returns): | |||
""" | |||
Determines the Information ratio of a strategy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to change doc string.
empyrical/stats.py
Outdated
@@ -1004,6 +990,208 @@ def cagr(returns, period=DAILY, annualization=None): | |||
|
|||
return ending_value ** (1. / no_years) - 1 | |||
|
|||
def capture(returns, factor_returns, period=DAILY): | |||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs a reference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What kind of reference ?
-
http://www.morningstar.com/InvGlossary/upside-downside-capture-ratio.aspx
-
http://www.investopedia.com/terms/u/up-market-capture-ratio.asp
Something like that? or am I misunderstanding what you mean by reference ?
@cgdeboer I won't do a full review yet as you label it as WIP but wanted to say that this looks fantastic. |
@twiecki I'll get this thing linted etc, and address your initial comments above. I'll remove the WIP title and notify you here once that's done. |
@twiecki alrighty, I still have some tests to write and verify, and I'm still conferring with some colleagues on the info ratio stuff (i.e how to best proceed with that). At my own peril, I'm removing the WIP, review away. |
@twiecki I think all of your comments have been addressed. Looks like ole Travis is just finishing up the CI testing (or maybe hung up on one build job). |
empyrical/stats.py
Outdated
Size of the rolling window in terms of the periodicity of the data. | ||
- eg window = 60, periodicity=DAILY, represents a rolling 60 day window | ||
""" | ||
return roll(returns, factor_returns, window=window, functions=up_capture, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newman !! Fixed it in 6efeee1
Alrighty @twiecki , addressed that last pluralized one. Let me know if you see anything else, could catch. |
Did the unittest catch it as well? |
Well. You know what, I don't think it did. Perhaps I need to write a functional test for each of the new methods. I sort of thought we'd rely on the individual stat tests for "financial" accuracy, and a sample of tests for the wrappers like I could see it going either way. What is your preference? |
Best to have it run through all code paths. |
Will add tests for all the methods. Back in a few. |
@cgdeboer Any update? |
@twiecki should have em all done this week. (My few referenced above turned from a few hours to a few days). Other stuff hopped in the way. Shooting for Thursday morning. |
OK, no rush obviously, just wondered if we could help. |
Most of the new tests are now written, just taking some extra time to double check the inputs vs expected. |
@twiecki tests added. 🏀 is in your court. |
empyrical/utils.py
Outdated
nanmin = np.nanmin | ||
nanargmax = np.nanargmax | ||
nanargmin = np.nanargmin | ||
nanmean = bn.nanmean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be reverted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted in dfe495f.
Great work, thanks @cgdeboer! |
Thanks !. @twiecki I'll keep an eye on https://pypi.python.org/pypi/empyrical/ for version update ? |
@cgdeboer New release should be up. |
Summary
Added a few helper methods in the utils file that allow us to stack some transformation logic together before calling an actual metric. For example, if we want to calculate the rolling up capture of a strategy vs its benchmark, we want to be able to do something like
[roll,up, capture]
whereroll
,up
,capture
, are each methods that perform either a series deconstruction (roll), a filter (up), or a calculation (capture). Doing it this why is more DRY than embedding roll and filter logic in each method, but does add some complication.I elected to add a variety of small methods to the
stats
file that wrap the complexity described above. This way we keep the API the user is used to calling the same. e.g there is actually a method calledrolling_up_capture
that takes the expected "returns, factor_returns" arguments. That function simply calls the more complex stacked function to get the results.Details
max_drawdown
. See note belownose_parameterized
to justparameterized
Not Done
information_ratio
per Remove Information Ratio #41empyrical
versionMax Drawdown
The
max_drawdown
code was not considering the first return in the array. Themax_return
array did not consider the starting value of 100 used in the function. In cases where ALL the returns in the array are negative, the max value in the array will be the 100 * (1+ the first negative return). For e.g if the first return in the array is-0.02
, then the max value in the array will be 98. This will cause max drawdowns to appear less severe than if they started at 100.The max drawdown code was masquerading as correct because the
negative_returns
Series in the test suite starts with a 0. (i.e they aren't all negative).I've corrected that by ensuring that the
cum_returns
array starts with a value that matches the base value.New Methods
Resolves #41 #46