Skip to content
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

ENH Add beta_fragility_heuristic and gpd_risk_estimates functions #42

Conversation

jeyoor
Copy link
Contributor

@jeyoor jeyoor commented Feb 23, 2017

Hi,

I thought it'd be interesting to add some risk metrics to empyrical for use in zipline/pyfolio.

Here's the papers I used.

I wasn't sure about whether or not docstrings and tests are required for subroutines. (I'm missing some docstrings and tests for subroutines of gpd_risk_estimates.)

If the above are required, please feel free to ping me back and I'll work on adding them.

Thanks!

Jeyan

@twiecki
Copy link
Contributor

twiecki commented Feb 27, 2017

Thanks @jeyoor, this looks great. I'll try to take a look next week (traveling).

@twiecki
Copy link
Contributor

twiecki commented Dec 21, 2017

Sorry @jeyoor, I completely dropped the ball on this. This looks great, if you solve the merge conflicts we can go ahead and merge this.

…y_and_gpd_risk_estimates

Conflicts:
	empyrical/__init__.py
	empyrical/stats.py
	empyrical/tests/test_stats.py
@jeyoor
Copy link
Contributor Author

jeyoor commented Jan 22, 2018

Hi @twiecki,

No worries; schedule pressure can be tough.

I solved the merge conflicts, but tests are currently failing.
I am planning to look into this some more when I have some time.

Tests run fine on quantopian:master so it must be something I added or changed, but it isn't clear to me right now exactly what's causing the tests to fail...

I'll keep you posted on the status.

Thanks!

- old tests work
- new tests still failing
- fixes TypeError: cannot concatenate a non-NDFrame object
- also, switch the functions from SIMPLE_STAT_FUNCS to FACTOR_STAT_FUNCS
@jeyoor
Copy link
Contributor Author

jeyoor commented Jan 22, 2018

Hi @twiecki,

I think everything passes now.

I wasn't sure if the new functions should be in SIMPLE_STAT_FUNCS or FACTOR_STAT_FUNCS, but they seemed to fit best in FACTOR_STAT_FUNCS so I moved them there.

I look forward to your feedback.

Thanks!

@@ -0,0 +1,71 @@
#
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file shouldn't show up.

@@ -0,0 +1,1616 @@
#
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file neither

@twiecki
Copy link
Contributor

twiecki commented Jan 22, 2018

Thanks for picking this back up! Looks like the merge wasn't quite successful yet. If everything fails, you could from a clean master you cherry-pick your commits, or recreate those functions.

@jeyoor
Copy link
Contributor Author

jeyoor commented Jan 22, 2018

Doh! Looks like I accidentally added some merge backup files.
EDIT: found and fixed another missed merge file.

How does it look now?
Went into the "Files Changed" tab and the diff looks cleaner...

@jeyoor
Copy link
Contributor Author

jeyoor commented Jan 29, 2018

Hi @twiecki,

How are the diffs looking after the most recent update?

Thanks!

The magnitude of the negative value indicates the size of
the potential loss.

seealso::
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put this under Notes (with --- underlining) at the bottom.

@@ -993,6 +995,325 @@ def cagr(returns, period=DAILY, annualization=None):
return ending_value ** (1. / no_years) - 1


def beta_fragility_heuristic(returns, factor_returns):
"""
Estimate fragility to drops in beta.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these line-breaks are a bit odd.

"""
Estimate fragility to drops in beta

seealso::
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same.

var_estimate - an estimate for the VaR for the given percentile
es_estimate - an estimate for the ES for the given percentile
"""
result = np.array([0.0, 0.0, 0.0, 0.0, 0.0])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

np.ones() or np.empty().

@gusgordon
Copy link
Contributor

Closing this as it's old. Feel free to reopen if this is still relevant.

@gusgordon gusgordon closed this Aug 22, 2019
@jeyoor
Copy link
Contributor Author

jeyoor commented Sep 10, 2019

@gusgordon ,
Thank you for the reminder on this!
I'm a bit slammed at the moment, but once I get some free time I plan to update the branch per the comments and ping you back.
Thanks again!
Jey

@jeyoor
Copy link
Contributor Author

jeyoor commented Sep 29, 2019

Moved to new pull request here #118

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants