OpenBBTerminal icon indicating copy to clipboard operation
OpenBBTerminal copied to clipboard

Stock menu refactoring for easier API usage

Open northern-64bit opened this issue 1 year ago • 3 comments

Description

This pr refactors the stocks menu for easier use of the api and is a part of the general API upgrade in combination of #2097 (and more future prs).

How has this been tested?

Will be tested soon

Checklist:

Others

  • [x] I have performed a self-review of my own code.
  • [x] I have commented my code, particularly in hard-to-understand areas.
  • [x] 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/....

northern-64bit avatar Jul 28 '22 09:07 northern-64bit

I'm reviewing it. will leave some comments.

Noticed that there are some commands in the stocks menu that's not yet refactored yet, such as candle or display_news

minhhoang1023 avatar Jul 28 '22 12:07 minhhoang1023

  • bullbear, get_dd, headlines command still have the ticker argument name.
image image image

minhhoang1023 avatar Jul 28 '22 12:07 minhhoang1023

  • bullbear, get_dd, headlines command still have the ticker argument name.

Will do, missed these since they were in the common file directory and not in the stocks directory

northern-64bit avatar Jul 28 '22 15:07 northern-64bit

@northern-64bit Excellent work! This is a beast of a PR 🚀 ✨

Some commands are missing default arguments, and some have not yet been refactored. I'm leaving it below.

  • api.stocks.ba.mentions, api.stocks.ba.sentiment, api.stocks.ba.watchlist, api.stocks.ba.trend, api.stocks.ins.act , api.stocks.ins.print_insider_data missing default value arguments image
image image
  • Commands in the root stocks menu are not yet refactored. Such as api.stocks.candle, api.stocks.display_news, api.stocks.get_news. Can you have a look?

minhhoang1023 avatar Aug 03 '22 22:08 minhhoang1023

@montezdesousa Can you take this one over and merge it in? Discussed with @Chavithra and @minhhoang1023.

JerBouma avatar Aug 08 '22 09:08 JerBouma

@montezdesousa Can you take this one over and merge it in? Discussed with @Chavithra and @minhhoang1023.

Ok I'll be on it, as soon as I finish economy menu

montezdesousa avatar Aug 08 '22 09:08 montezdesousa