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

New Feature Financial Analysis, checks Yahoo if no data at Alpha Advantage. #1984

Merged
merged 38 commits into from
Jun 29, 2022

Conversation

pauljsymonds
Copy link
Contributor

Description

  • Summary of the change - Added a feature to go to Yahoo Finance for Cash flows, Income Statement and Balance Sheet when data not in Alpha Advantage, (Covers UK tickers)

  • Relevant motivation and context. - Useful for those in UK looking at FTSE data.

  • List any dependencies that are required for this change. - import ssl, numpy and from bs4 import BeautifulSoup

How has this been tested?

Tested:

stocks
load SNR.L
fa
balance
cash
income

all returned data

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/....

… and cashflow, if tickers not found in Alpha Vantage. Should cover all UK (only tested with .L appended to tickers).
@colin99d colin99d self-requested a review June 24, 2022 11:42
@colin99d colin99d added the enhancement Enhancement label Jun 24, 2022
@colin99d
Copy link
Contributor

Awesome idea! I can review this tonight or tomorrow.

pauljsymonds and others added 4 commits June 24, 2022 18:20
… and cashflow, if tickers not found in Alpha Vantage. Should cover all UK (only tested with .L appended to tickers).
Copy link
Contributor

@colin99d colin99d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice pr! I really like adding in yahoo finance when alpha vantage fails. I noticed two issues:

  • Date in the headers is formatted DD/MM/YYYY, but we stick to YYYY/MM/DD terminal-wide
  • It looks that if while I am in the fa menu I load SNR.L and then load GME and then load SNR.L again none of the commands work anymore and they all produce errors

@colin99d
Copy link
Contributor

Also, this is optional but some people may want to choose yahoo finance over alpha vantage when both are available. Adding a source option in the controller would be helpful for this. If you need any help here let me know.

@pauljsymonds
Copy link
Contributor Author

pauljsymonds commented Jun 28, 2022

I've made the changes you suggested to the dates and separated the source. I used this format as that is what is returned:

2017-06-30

I've removed the code that calls Yahoo from the AV so no data is returned to keep it separate.

I have not successfully been able to get the --source flag working so cannot test.

I noticed when in the terminal and -s is passed it returns the sources (intellisense) yet -s is for dates. Not sure where that is coming from.

Copy link
Contributor

@colin99d colin99d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Once tests pass lets merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants