-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fix: Remove hardcoded keys in Analysis #2194
Conversation
Surely it would be quicker and simpler to simply store the keys as lower case strings |
I am aiming to keep backwards compatibility |
And what if Yahoo changes another key? Better if most hardcodings go - just unpack the json and use what Yahoo provides as much as possible. Try this:
|
Yes, especially as they already have added one: 90 days |
34512be
to
eae74ae
Compare
Do you mind refactoring the other fetch functions? |
eae74ae
to
a9859e2
Compare
Done. Edited PR description to show changes to end result |
Refactor some fetch functions (duplicate logic). Fix failing unit tests.
722a51e
to
508afca
Compare
Removed hard coded keys in the Analysis scraper functions to protect against Yahoo changing the keys in the future.
Changes
End result is more or else the same except:
Earnings Estimate
Before:
After:
Revenue Estimate
Before:
After:
Earnings History
Before:
After:
EPS Trend
Before:
After:
EPS Revisions
Before:
After:
Growth Estimates
Before:
After:
Closes #2188