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

Feature/news sentiment #4339

Merged
merged 54 commits into from
Jun 9, 2023
Merged

Conversation

Sai-132
Copy link
Contributor

@Sai-132 Sai-132 commented Feb 27, 2023

Description

Newssentiment is a function that displays top news articles and associated sentiment for stock tickers provided by media aggregator Onclusive, through the alternative data platform Althub

  • newssentiment is in the main menu of your terminal

  • newssentiment/show is used to get the news articles and the associated sentiment

  • usage:

      [show, -t,--ticker, -sd, --start_date, -ed, --end_date, -d, --date, -l,--limit,-o,--offset, --export]
      
      These all are optional arguments:
    
      -t, --ticker [ TICKER ]
               Ticker to analyze (default: None)
      -sd, --start_date [ START DATE ]
               The starting date (format YYYY-MM-DD) to search articles from (default: False)
      -ed, --end_date [ END DATE ]
               The end date (format YYYY-MM-DD) to search articles up to (default: False)
      -d, --date [ DATE ]
               Show the article data on this day (format YYYY-MM-DD). (default: False)
               If you use the date argument start_date and end_date arguments will be ignored.
      -l, --limit [ LIMIT ]
               Number of articles to be displayed (default: 100)
      -o, --offset [ OFFSET ]
               The offset indicates the starting position of article records (default: 0)
      --export [ EXPORT ]
               Export raw data into csv, json, xlsx (default: )
    
      Note: if a date or a date range is not specified, articles from the last five business days are considered.
    
  • example:

      show -t AAPL -sd 2023-01-01 -ed 2023-01-30 -l 50 -o 200
    

How has this been tested?

  • Please describe the tests that you ran to verify your changes.
  • Provide instructions so we can reproduce.
  • Please also list any relevant details for your test configuration.
  • Make sure affected commands still run in terminal
  • Ensure the SDK still works
  • Check any related reports

Checklist:

Others

  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.

@reviewpad reviewpad bot added the feat M Medium T-Shirt size feature label Feb 27, 2023
@deeleeramone
Copy link
Contributor

Hey, thanks for the PR! A few notes on this:

  1. This belongs in the Stocks menu because it is specific to Stocks.
  2. This doesn't warrant its own sub-menu, it is one function, and is more suited to be in the Behavioural Analysis or Discovery sub-menus.
  3. Arguments for Terminal functions (argparse) need to follow the convention: - for one letter only, -- for everything else. -s & --start_date, -e & --end_date.
  4. The nested completer is not working for the function's arguments.
  5. The name of the function does not correlate with what the function does. show is used as an argument elsewhere, a better handle for the function would be: ns, short for news_sentiment.
  6. Naming needs to be followed. news_sentiment instead of NewsSentiment. onclusivedata_controller, etc. No capitals in the file names; OnclusiveData would be used as a class object name, and not a file or function.
  7. The folder has no __init__.py file, nothing can be imported from the folder. This should be moved to a sub-menu anyways, the model and view files are the only ones which need to travel.
  8. Most importantly, It does not appear to work.
    Screenshot 2023-02-27 at 5 10 21 PM

@Sai-132
Copy link
Contributor Author

Sai-132 commented Feb 28, 2023

As per the instructions above, I have updated the folder structure and created ns (News Sentiment) files under the Discovery sub-menus.

image

  • To access Althub APIs, you just need to set the environment variable (OPENBB_ALTHUB_API_TOKEN).

  • In our previous email conversations, we sent an API_TOKEN. You can access our APIs using that token.

@deeleeramone
Copy link
Contributor

This has not been added to the /keys menu, and instructions in the documentation pages need to be included for obtaining any API token.

To access Althub APIs, you just need to set the environment variable (OPENBB_ALTHUB_API_TOKEN).

Screenshot 2023-02-28 at 8 37 00 AM

Error when handling no key entered:

2023 Feb 28, 11:38 (🦋) /stocks/disc/ $ ns

OPENBB_ALTHUB_API_TOKEN not defined. Set API Keys in ~/.openbb_terminal/.env or under keys menu.
Error: 'NoneType' object has no attribute 'dropna'

Please run the testing suite locally and clear all linting errors:

************* Module openbb_terminal.stocks.discovery.news_sentiment_model
news_sentiment_model.py 63: C0301 Line too long (144/140) (line-too-long)
news_sentiment_model.py 63: W1309 Using an f-string that does not have any interpolated variables (f-string-without-interpolation)
news_sentiment_model.py 9: C0411 standard import "import datetime" should be placed before "import requests" (wrong-import-order)
news_sentiment_model.py 10: C0411 standard import "import os" should be placed before "import requests" (wrong-import-order)
news_sentiment_model.py 5: W0611 Unused import json (unused-import)
news_sentiment_model.py 8: W0611 Unused numpy imported as np (unused-import)
news_sentiment_model.py 9: W0611 Unused import datetime (unused-import)
news_sentiment_model.py 10: W0611 Unused import os (unused-import)

-----------------------------------
Your code has been rated at 7.89/10

@deeleeramone
Copy link
Contributor

You should also update this branch to be current with origin/develop

@deeleeramone
Copy link
Contributor

Type error in the controller, string is not a bool.

Please review the help messages for each argument, and correct the grammar.

Screenshot 2023-02-28 at 8 49 29 AM

@Sai-132
Copy link
Contributor Author

Sai-132 commented Mar 1, 2023

I have gone through all the above notes and updated the code accordingly.

I updated the origin/develop branch also.

Screenshot 2023-03-01 171229

I updated the type for all arguments.

@Sai-132
Copy link
Contributor Author

Sai-132 commented Mar 1, 2023

I have updated the API key also

image

@deeleeramone
Copy link
Contributor

I have updated the API key also

image

Please resolve the merge conflicts, the branch has not been updated to be in sync with the develop branch until this has happened.

@reviewpad reviewpad bot added feat L Large T-Shirt size Feature and removed feat M Medium T-Shirt size feature labels Mar 8, 2023
@Sai-132
Copy link
Contributor Author

Sai-132 commented Mar 8, 2023

Can you please check now
I have resolved all the conflicts

@deeleeramone
Copy link
Contributor

Can you please check now
I have resolved all the conflicts

Please resolve all conflicts.

Screenshot 2023-03-08 at 5 15 23 PM

@Sai-132
Copy link
Contributor Author

Sai-132 commented Mar 9, 2023

resolved conflicts in the config_terminal file

@deeleeramone
Copy link
Contributor

resolved conflicts in the config_terminal file

Not sure what you did there, but now the keys menu does not work. This error does not occur on the origin/develop branch. There should be no reason for you to have edited this file, and you should revert any changes made.

Screenshot 2023-03-09 at 11 16 15 AM

@@ -20,6 +20,8 @@
controllers as ctrl,
models as model,
)
from openbb_terminal import feature_flags as obbff
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these items being added? Line 2 of the file says explicitly that this file is auto generated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While Running this Command "python generate_sdk.py sort"
These lines are automatically generated.

@@ -413,6 +413,7 @@ def call_stocks(self, _):

self.queue = self.load_class(StocksController, self.queue)


Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we adding an empty new line here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This empty new line may have been added by mistake.

@Sai-132 Sai-132 force-pushed the feature/news_sentiment branch from 4bc7abe to 9beba4c Compare May 17, 2023 10:37
@reviewpad reviewpad bot added feat XS Extra small feature and removed feat M Medium T-Shirt size feature labels May 17, 2023
@Sai-132 Sai-132 reopened this May 17, 2023
@reviewpad reviewpad bot added feat S Small T-Shirt size Feature and removed feat XS Extra small feature labels May 17, 2023
@Sai-132
Copy link
Contributor Author

Sai-132 commented May 17, 2023

Fixed all merge conflicts and it is accessible without an API key.

@Sai-132
Copy link
Contributor Author

Sai-132 commented May 22, 2023

@minhhoang1023 We've already updated the code, fixed the conflicts, and your terminal users can access it without an API key.

@jmaslek jmaslek enabled auto-merge June 9, 2023 16:11
@jmaslek jmaslek added this pull request to the merge queue Jun 9, 2023
Merged via the queue into OpenBB-finance:develop with commit 6784a9a Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat S Small T-Shirt size Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants