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

Update functionality with FinanceDatabase v2, depreciate stocks/sia, fix exe --example and fix stocks/fa/dcf #4319

Merged
merged 38 commits into from
Feb 27, 2023

Conversation

JerBouma
Copy link
Contributor

@JerBouma JerBouma commented Feb 23, 2023

Description

Basically what changed in this new version of the FinanceDatabase is the following:

  • Package itself is object orientated, needs initialisation (collect data first) but gets the job done in a much quicker fashion. For example a file with 180k tickers is loaded within 2 seconds and from there on everything is done locally and really quick.
  • The Database itself is now CSV files meaning it is much easier to edit the files. No longer thousands of JSON files that have the same data multiple times.
  • All files loaded into the package are compressed down versions of Pickles with compression method xz. It shrinks the original JSON file of 120mb to 10mb, see this file. Auto-updated with GitHub Actions.
  • Querying is easier with .search using kwargs.
  • Equities now follows GICS sectors, industry groups and industries.
  • ETFs and Funds have their categories reduced and grouped. Furthermore, families (e.g. Vanguard) now is grouped better (there were 5 different versions of their name).
  • Multiple corrections within the entire database.
  • Revamp of the whole thing to entice people to help out and update (also given that the CSVs now exist). Whether this actually happens is of course yet to be seen.

For OpenBB Terminal this means:

  • stocks/searchis faster.
  • etf search functions are faster and updates are more frequent.
  • stocks/sia is less buggy with less dependency on Yahoo Finance. However, this PR also retires this menu for the time being. This is because it's currently functionality is better resolved within stocks/ca and through routines.
  • Some minor misc updates, making code more efficient and requiring far less calls to the web (specifically GitHub).
  • Also fixed stocks/fa/dcf since it relied partly on FinanceDatabase so I extended that to replace Yahoo Finance functionality.

Fixes #4225

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 23, 2023
@JerBouma JerBouma changed the title Update functionality with FinanceDatabase v2 Update functionality with FinanceDatabase v2 and fix stocks/fa/dcf Feb 23, 2023
@JerBouma JerBouma changed the title Update functionality with FinanceDatabase v2 and fix stocks/fa/dcf Update functionality with FinanceDatabase v2, depreciate stocks/sia and fix stocks/fa/dcf Feb 23, 2023
@JerBouma JerBouma changed the title Update functionality with FinanceDatabase v2, depreciate stocks/sia and fix stocks/fa/dcf Update functionality with FinanceDatabase v2, depreciate stocks/sia, fix exe --example and fix stocks/fa/dcf Feb 23, 2023
@JerBouma JerBouma marked this pull request as ready for review February 23, 2023 15:12
@JerBouma
Copy link
Contributor Author

Done, not entirely sure what's with the infinity mark regarding files 😅

@hjoaquim
Copy link
Contributor

@JerBouma did you use poetry to update financedatabase dependency?

@JerBouma
Copy link
Contributor Author

@hjoaquim I didn't, it failed on me when I did that 😅

@hjoaquim
Copy link
Contributor

Sorry to comment like this but, can't comment directly on VSCode:

  • Is this function being used anywhere or we can deprecate it openbb_terminal.stocks.discovery.financedatabase_view.show_equities?
  • On dcf_view is there a way to remove this yf.Ticker(symbol).fast_info (similarly to what was done for the info)?

@JerBouma
Copy link
Contributor Author

Sorry to comment like this but, can't comment directly on VSCode:

  • Is this function being used anywhere or we can deprecate it openbb_terminal.stocks.discovery.financedatabase_view.show_equities?
  • On dcf_view is there a way to remove this yf.Ticker(symbol).fast_info (similarly to what was done for the info)?

First one is indeed not used. I just updated it for the sake of it. Within the dcf_view I actually believe that already has no value to begin with.

@JerBouma
Copy link
Contributor Author

JerBouma commented Feb 24, 2023

Just a headsup, this PR depreciates the stocks/sia menu from the OpenBB Terminal and deletes sia paths from the OpenBB SDK. Given that we do not maintain it anymore within the OpenBB Terminal for the time being it wouldn't make sense to keep it around in the OpenBB SDK. Furthermore, it also gets rid of the documentation pages related to it.

See: a9a27b6

@JerBouma
Copy link
Contributor Author

JerBouma commented Feb 24, 2023

Fixed the fact that it didn't actually search in short_name and the index properly. It does now with 7375b93 so you can do the following:

(🦋) /stocks/ $ search tsla --all-exchanges

                                                      Companies found on term tsla                                                      
┏━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━┓
┃           ┃ Name                        ┃ Country       ┃ Sector                 ┃ Industry Group           ┃ Industry    ┃ Exchange ┃
┡━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━┩
│ TSLA-U.TI │ Tesla, Inc.                 │ United States │ Consumer Discretionary │ Automobiles & Components │ Automobiles │ TLO      │
├───────────┼─────────────────────────────┼───────────────┼────────────────────────┼──────────────────────────┼─────────────┼──────────┤
│ TSLA.BA   │ Tesla, Inc.                 │ United States │ Consumer Discretionary │ Automobiles & Components │ Automobiles │ BUE      │
├───────────┼─────────────────────────────┼───────────────┼────────────────────────┼──────────────────────────┼─────────────┼──────────┤
│ TSLA.MI   │ Tesla, Inc.                 │ United States │ Consumer Discretionary │ Automobiles & Components │ Automobiles │ MIL      │
├───────────┼─────────────────────────────┼───────────────┼────────────────────────┼──────────────────────────┼─────────────┼──────────┤
│ TSLA.MX   │ Tesla, Inc.                 │ United States │ Consumer Discretionary │ Automobiles & Components │ Automobiles │ MEX      │
├───────────┼─────────────────────────────┼───────────────┼────────────────────────┼──────────────────────────┼─────────────┼──────────┤
│ TSLA      │ Tesla, Inc.                 │ United States │ Consumer Discretionary │ Automobiles & Components │ Automobiles │ NMS      │
├───────────┼─────────────────────────────┼───────────────┼────────────────────────┼──────────────────────────┼─────────────┼──────────┤
│ TSLA.VI   │ Tesla, Inc.                 │ United States │ Consumer Discretionary │ Automobiles & Components │ Automobiles │ VIE      │
├───────────┼─────────────────────────────┼───────────────┼────────────────────────┼──────────────────────────┼─────────────┼──────────┤
│ TSLA34.SA │ Tesla, Inc.                 │ United States │ Consumer Discretionary │ Automobiles & Components │ Automobiles │ SAO      │
├───────────┼─────────────────────────────┼───────────────┼────────────────────────┼──────────────────────────┼─────────────┼──────────┤
│ TSLAC.BA  │ Tesla, Inc.                 │ United States │ Consumer Discretionary │ Automobiles & Components │ Automobiles │ BUE      │
├───────────┼─────────────────────────────┼───────────────┼────────────────────────┼──────────────────────────┼─────────────┼──────────┤
│ TSLAB.BA  │ Tesla, Inc.                 │ nan           │ Consumer Discretionary │ Automobiles & Components │ Automobiles │ nan      │
├───────────┼─────────────────────────────┼───────────────┼────────────────────────┼──────────────────────────┼─────────────┼──────────┤
│ JS3HN9.TI │ GSF BIDU/NIO/TSLA CC 310325 │ nan           │ nan                    │ nan                      │ nan         │ TLO      │
└───────────┴─────────────────────────────┴───────────────┴────────────────────────┴──────────────────────────┴─────────────┴──────────┘

@JerBouma
Copy link
Contributor Author

Small fix here so that it doesn't crash if you have no ticker loaded.

image

@jmaslek Let me know if I need to give you a tour what I changed due to infinite files.

@jmaslek
Copy link
Collaborator

jmaslek commented Feb 25, 2023

Small fix here so that it doesn't crash if you have no ticker loaded.

image

@jmaslek Let me know if I need to give you a tour what I changed due to infinite files.

found what infinite means to github:
Screenshot 2023-02-25 at 6 13 54 PM

@jmaslek
Copy link
Collaborator

jmaslek commented Feb 25, 2023

etf/screener_controller can be deleted (with associated tests)

@jmaslek
Copy link
Collaborator

jmaslek commented Feb 25, 2023

On stocks/load I am not getting autocomplete options with nay of the choices :(

@jmaslek
Copy link
Collaborator

jmaslek commented Feb 25, 2023

On stocks/load I am not getting autocomplete options with nay of the choices :(

Related to this: when I go into stocks I get this message.

2023 Feb 25, 18:31 (🦋) / $ stocks

Note: Some datasets from GitHub failed to load. This means that the `search` command and the /stocks/sia menu will not work. If other commands are failing please check your internet connection or communicate with your IT department that certain websites are blocked.

Am at home. Have always done this from home. (Probably also want to get rid of the stocks/sia reference there)

@JerBouma
Copy link
Contributor Author

You get that because you are not on the latest version of the package, v2.0.6. Thats also why no auto complete in search.

@jmaslek
Copy link
Collaborator

jmaslek commented Feb 26, 2023

You get that because you are not on the latest version of the package, v2.0.6. Thats also why no auto complete in search.

Hm. Alright. Poetry install is giving 2.0.4 so we need to bump that.

@jmaslek
Copy link
Collaborator

jmaslek commented Feb 26, 2023

You get that because you are not on the latest version of the package, v2.0.6. Thats also why no auto complete in search.

Hm. Alright. Poetry install is giving 2.0.4 so we need to bump that.

fixed

@JerBouma
Copy link
Contributor Author

@jmaslek

Updated package to v2.0.7 (saw minor mistake in my own code) and removed etf/screener. Anything else before merge?

@jmaslek
Copy link
Collaborator

jmaslek commented Feb 27, 2023

@jmaslek

Updated package to v2.0.7 (saw minor mistake in my own code) and removed etf/screener. Anything else before merge?

Just make sure tests are passing

pyproject.toml Outdated Show resolved Hide resolved
Copy link
Collaborator

@jmaslek jmaslek left a comment

Choose a reason for hiding this comment

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

small changes. lgtm

@jmaslek jmaslek enabled auto-merge February 27, 2023 16:52
@jmaslek jmaslek added this pull request to the merge queue Feb 27, 2023
Merged via the queue into develop with commit 696027b Feb 27, 2023
@colin99d colin99d deleted the feature/finance-database-update branch February 27, 2023 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat M Medium T-Shirt size feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Stocks/DCF - Function is broken
3 participants