-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Refractor stocks/fa
by incorporating stocks/dd
and making the menu less reliant on Yahoo Finance
#4055
Conversation
So one thing to note, if you look at yfinance 0.2.9, it seems like they replaced .info with .fast_info, so does this work if you locally run with |
I was playing around with fast_info and it doesn't seem to include all info so I sticked with |
Aight. I wasn't sure if info was being deprecated and would break |
stocks/fa
with less Yahoo Financestocks/fa
by incorporating stocks/dd
and making the menu less reliant Yahoo Finance
Updated but 100% tests won't work flawlessly. That's a task for next week. |
Wait until we bump yf. That will (likely) need tests to be rerun anyways |
Does it make more sense to do things under Also you need to make sure you change all of the SDK paths and docstring hints/examples for whatever we go with |
Fundamental Analysis is really the whole area of examining the business model, management, competitive advantage, governance, financial statements and ratios, intrinsic value estimations and future estimations. That's in essence what you see in this menu. Yes, it would also include a comparison analysis and market analysis but that's secondary topics. I can not think of a better name to capture what's inside the menu than "Fundamental Analysis". The act of due diligence is just so much more and can also expand technical analysis, quantitative analysis, market analysis, economy analysis and more as in essence doing DD is a fully fledged investigation before you are making a decision to make alterations to your portfolio. There are also multiple areas here, financial due diligence, legal due diligence, commercial due diligence and more. With all of those areas, you essentially touch upon everything about the company. So, renaming to dd would be odd since then it should also include all other aspects. For example, see these sources:
Thanks for the reminder, need to look into that as well. |
stocks/fa
by incorporating stocks/dd
and making the menu less reliant Yahoo Financestocks/fa
by incorporating stocks/dd
and making the menu less reliant on Yahoo Finance
@jmaslek I lack technological know-how to fix these merge conflicts. I gave it a shot but if I continue the chances are pretty high I'll just overwrite the wrong code. Can you look into this? |
Probably wont have time to check for a few days. But it looks like those are just some files that you moved to the fa folder? So you just may need to delete them. And then all the yaml are just tests you can delete and rewrite in the fa folder. |
I wouldn't redirect. Just say its been deprecated and functions are integrated into fa. If we redirect then it still works and when we drop it completely, will cause issues. In a couple weeks we just drop that call_dd completely (add a TODO :) ) |
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.
And the sdk needs to be remapped before merge. Requesting changes so that I remember this
Changed it! |
sdk ? |
you need to remap all the functions that were stocks.dd.{} to stocks.fa.{} |
But where do you do that |
I seeeeee. That is the part not updated. It is in miscellaneous/library/trail_map.csv |
0708d3e
to
d8765cb
Compare
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.
The dependency tree has been changed. Please update requirements.txt and requirements-full.txt
Soo this should be it, fixed the OpenBB SDK stuff and I've fixed a bunch of |
No clue what this is about. |
ignore |
will review shortly. |
@jmaslek Lol tests fail at thought of the day, can you take a look what to do with those? |
Done. |
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.
Everything is working on my end in terminal and in SDK. Thing this organization makes a ton more sense.
Aight, I'll merge it when it's done with the tests. I'll make sure to check the commands over the coming days if it all works well. This all came from the fact that during presentations I kept typing "Now go to the dd menu", "Now go back to the fa" menu 😝 |
The
stocks/dd
menu shares a strong similarity withstocks/fa
up to the point that it makes sense to combine the two. This led to the "New" menu as depicted below.Next to that, since Yahoo Finance (specifically
yfinance
) tends to break every now and then, this PR moves it to not be the main source anymore for most of the functionalities instocks/fa
and groups functionalities where relevant.Furthermore:
stocks/quote
now uses FinancialModelingPrep as main source and not Yahoo Finance, taken that function fromstocks/fa/quote
and moved it out ofstocks/fa
altogether.stocks/filings
is moved tostocks/fa
EODHD
requires a paid plan instocks/fa
which is now mentioned and documentation is updated to include referral link we also mention here.See the change to the
fa
menu:Old
New