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

Adding everything from Risk Metrics PR based on new main branch #1666

Merged
merged 14 commits into from
Apr 15, 2022

Conversation

northern-64bit
Copy link
Contributor

Description

Adding sh, so, om commands. This pr is based on the changes from #1515 that broke due to renameing.

How has this been tested?

Using the commands

Checklist:

Others

  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • My code passes all the checks pylint, flake8, black, ... To speed up development you should run pre-commit install.
  • New and existing unit tests pass locally with my changes. You can test this locally using pytest tests/....

@northern-64bit northern-64bit added the feat M Medium T-Shirt size feature label Apr 10, 2022
@JerBouma
Copy link
Contributor

JerBouma commented Apr 11, 2022

Hey, thanks for the PR! It seems that the -w argument doesn't accept any integers. Perhaps it gets saved as a string?

Screenshot 2022-04-11 at 10 38 23

Could you also add a little bit more documentation about these ratios on the website? What I am mostly looking for is a quick explanation what the Omega Ratio is for example. You already link to an article within the code but it would be helpful if that explanation is briefly given within the Docs as well.

Furthermore, could you add in an example to each command? You can do so by going into your own Fork GitHub location and directly editing the .md files. This allows you to drag and drop images thus you can simply run the command, save the image and drop it into the .md

@JerBouma JerBouma self-requested a review April 11, 2022 08:42
@northern-64bit
Copy link
Contributor Author

Could you also add a little bit more documentation about these ratios on the website? What I am mostly looking for is a quick explanation what the Omega Ratio is for example. You already link to an article within the code but it would be helpful if that explanation is briefly given within the Docs as well.

Furthermore, could you add in an example to each command? You can do so by going into your own Fork GitHub location and directly editing the .md files. This allows you to drag and drop images thus you can simply run the command, save the image and drop it into the .md

Should I also add a quick explanation of the ratio:s in the command help part (-h)? I'm a bit unsure how detailed it should be.

@jmaslek
Copy link
Collaborator

jmaslek commented Apr 14, 2022

Should I also add a quick explanation of the ratio:s in the command help part (-h)? I'm a bit unsure how detailed it should be.

Would be helpful to have some formulas and descriptions in the Hugo (which is what @JerBouma is suggesting)

@JerBouma
Copy link
Contributor

@northern-64bit

The -h text doesn't have to be so precise. You could say for the sharpe ratio for example "Determines the Sharpe Ratio over time which is the portfolio return minus the risk free return divided by the portfolio's standard deviation."

In the Hugo docs you could give the formula, provide (a little bit of) background information and show one example. It doesn't have to be an entire paragraph and feel free to just link to your source, happy to approve this soon :)

@northern-64bit
Copy link
Contributor Author

The -h text doesn't have to be so precise. You could say for the sharpe ratio for example "Determines the Sharpe Ratio over time which is the portfolio return minus the risk free return divided by the portfolio's standard deviation."

In the Hugo docs you could give the formula, provide (a little bit of) background information and show one example. It doesn't have to be an entire paragraph and feel free to just link to your source, happy to approve this soon :)

I've now added a description to the hugo. After having thought about it I think that the command help description is good as it is and instead I made the hugo documentation more precise. The command help shouldn't clutter up the entire terminal and be easy to use. Looking forward towards having this merged 👍

@DidierRLopes DidierRLopes merged commit fd2839c into main Apr 15, 2022
@northern-64bit northern-64bit deleted the risk-metrics2 branch April 15, 2022 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat M Medium T-Shirt size feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants