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

[Bug] Fixes the stocks ca cashflow command #2486

Merged
merged 3 commits into from
Sep 5, 2022

Conversation

northern-64bit
Copy link
Contributor

Description

Fixes issue #2478

How has this been tested?

Using the command with various tickers and inputs.

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

@@ -737,7 +737,8 @@ def call_cashflow(self, other_args: List[str]):
dest="s_timeframe",
type=str,
default=None,
help="Specify yearly/quarterly timeframe. Default is last.",
help="Specify year/quarter of the cashflow statement to be retrieved. The format for year is YYYY and for "
Copy link
Collaborator

Choose a reason for hiding this comment

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

Technically its not quarter, since specifying year quarter would be something like 2021/Q3, but this requires the quarter end date. Not sure if there is an easy workaround other than having a list of all possible dates ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that different firms publish their quarterly report on different dates, so you cannot really have autocomplete with this.
One option is to change the quarter flag to having int representing the quarter to retrieve, then we only need to enter the year in the timeframe flag. However this requires extensive refactoring and other commands use the same functions too, thus they would also have to be changed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah this will require some thinking and we should probably also move away from using this scraping source moving forward. But I am good with how it is now.

@jmaslek jmaslek added the bug Fix bug label Sep 4, 2022
@colin99d colin99d merged commit 9c5ad13 into OpenBB-finance:main Sep 5, 2022
@northern-64bit northern-64bit deleted the fix-2478 branch September 5, 2022 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fix bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants