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

Feature/beta parameter #84

Merged

Conversation

PietropaoloFrisoni
Copy link
Collaborator

Adding computation of the beta parameter for the portfolio. In the Capital Asset Pricing Model (CAPM), the beta parameter defines how risky the portfolio is compared to the market.

Important: I could not run the old tests due to the usual quandl error (although I obtained my API key). I still don't know what's happening, but I'm working on it. The old tests should be run as a sanity check before merging the pull request.

However, the additional feature is written so that all the previous codes and tests should still work: suppose the argument "market_index" is not explicitly passed to the "build_portfolio" function. In that case, the Market class instance is not created, and the beta parameter of the portfolio is not computed or printed in properties.

@fmilthaler if there's something you would like me to change, please tell me.

All the best

@fmilthaler
Copy link
Owner

Thank you so much for your past contributions and this big one! It's a big help.

Don't worry about the quandl error, you can ignore those for the time being.

I'll have a closer look at it over the next few days. Once this is merged, I'll create a new release for FinQuant.

@fmilthaler fmilthaler self-requested a review July 9, 2023 14:40
@PietropaoloFrisoni
Copy link
Collaborator Author

Excellent, thank you! Please take your time and let me know if I should change something.

Best

@fmilthaler fmilthaler force-pushed the feature/beta-parameter branch from d1a50f0 to f08dc69 Compare July 11, 2023 11:15
@fmilthaler
Copy link
Owner

Great addition @PietropaoloFrisoni. Mainly just some minor changes, e.g. adding comment or removing empty line.

With the introduction of the class Market, which has a huge overlap with Stock, we should rather come up with a class Asset, define common methods in there, and then have Stock and Market inherit from Asset. That would be good. However, I'm fine to have that change done in a separate PR. Your decision, if you want to do it later, then please open an issue for that.

One additional change would be great for this PR: Could you add this as an example? You could make some modifications to example/Example-Build-Portfolio-from-web.py. Currently, that example uses quandl to retrieve data. Since quandl is basically dead, feel free to also change that example to use yfinance instead, but leave all the other mentions of quandl for now. Removing quandl properly should be done in another PR (some day in the future) ;)

finquant/portfolio.py Show resolved Hide resolved
finquant/market.py Show resolved Hide resolved
finquant/portfolio.py Outdated Show resolved Hide resolved
finquant/portfolio.py Outdated Show resolved Hide resolved
finquant/portfolio.py Outdated Show resolved Hide resolved
finquant/portfolio.py Show resolved Hide resolved
finquant/portfolio.py Show resolved Hide resolved
finquant/portfolio.py Show resolved Hide resolved
PietropaoloFrisoni and others added 6 commits July 11, 2023 15:45
typo in comment describing Market instance in the portfolio

Co-authored-by: Frank Milthaler <fmilthaler@users.noreply.github.com>
Adding decorator to set the market index

Co-authored-by: Frank Milthaler <fmilthaler@users.noreply.github.com>
removing empty line

Co-authored-by: Frank Milthaler <fmilthaler@users.noreply.github.com>
@PietropaoloFrisoni
Copy link
Collaborator Author

@fmilthaler Hi, I'm following your suggestions and updating with commits. However, the checks are currently failing due to quandl. At line 38 of the build (3.11) test, I see that:

quandl.errors.quandl_error.LimitExceededError: (Status 429) (Quandl Error QELx01) You have exceeded the anonymous user limit of 50 calls per day. To make more calls today, please register for a free Nasdaq Data Link account and then include your API key with your requests.

I may be missing something, but it looks like this is the source of the error. Any idea how to solve this?

@PietropaoloFrisoni
Copy link
Collaborator Author

By the way, I agree it is very reasonable to create an Asset parent class for Stock and Market. As I replied in one of the commits, I think the easiest and most straightforward approach is creating an issue to work on in the future. I will create the issue myself after this PR has been merged (hopefully after fixing the tests).

@PietropaoloFrisoni
Copy link
Collaborator Author

PietropaoloFrisoni commented Jul 11, 2023

@fmilthaler Great, tests work now. I'll work on the example/Example-Build-Portfolio-from-web.py as soon as possible (tomorrow at the latest)

@fmilthaler
Copy link
Owner

@fmilthaler Great, tests work now. I'll work on the example/Example-Build-Portfolio-from-web.py as soon as possible (tomorrow at the latest)

Thank you, that would be most helpful for the users (including myself) to see how to use it. :)

@PietropaoloFrisoni
Copy link
Collaborator Author

Hi @fmilthaler, I updated the example as well. Please tell me if there's something you would like to change.

Best

@fmilthaler
Copy link
Owner

fmilthaler commented Jul 12, 2023

@PietropaoloFrisoni Amazing work, I really appreciate all the work you put in here!!! You are a star. Just one minor fix I flagged. Once that is in, this can be merged (and another release is going to happen) :)

@PietropaoloFrisoni
Copy link
Collaborator Author

That's way too kind. Thank you so much. Maybe I'm missing something, as I cannot see the flagged minor issue. Could you please point that out (again)?

@fmilthaler
Copy link
Owner

fmilthaler commented Jul 12, 2023

Could you please point that out (again)?

That is weird. I commented on it again and tagged you. In case it still does not show up for you:
file: example/Example-Build-Portfolio-from-web.py, line 8. Change that line to:

# This example only focuses on how to use `build_portfolio()` to get an instance of `Portfolio` by providing a few items of information that is passed on to `quandl`/`yfinance`. For a more exhaustive description of this package and example, please try `Example-Analysis` and `Example-Optimisation`.

Only change in that (long) line is the addition of items of behind providing a few. :)

@PietropaoloFrisoni
Copy link
Collaborator Author

Done. Thanks for pointing that out!

example/Example-Build-Portfolio-from-web.py Outdated Show resolved Hide resolved
@fmilthaler fmilthaler merged commit 5068ae0 into fmilthaler:master Jul 13, 2023
@PietropaoloFrisoni PietropaoloFrisoni deleted the feature/beta-parameter branch July 13, 2023 06:30
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.

2 participants