Closed PietropaoloFrisoni closed 1 year ago
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.
Excellent, thank you! Please take your time and let me know if I should change something.
Best
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) ;)
@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.quandlerror.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?
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).
@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 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. :)
Hi @fmilthaler, I updated the example as well. Please tell me if there's something you would like to change.
Best
@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) :)
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)?
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
. :)
Done. Thanks for pointing that out!
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