Return a dataframe from stocks search, removed export to file system (#3923) #4193
Conversation
| if export_to_file: | ||
| export_data( | ||
| export, | ||
| os.path.dirname(os.path.abspath(__file__)), | ||
| "search", | ||
| df, | ||
| sheet_name, | ||
| ) |
There was a problem hiding this comment.
I am not following the logic of this export_to_file. If export=="", which it is by default, nothing gets saved.
The way we have our sdk is that exporting should not be handled through this. Returning a df allows you to save, ie openbb.stocks.search("Apple").to_csv("path/to/file"
There was a problem hiding this comment.
Ah, you are right. The underlying function does have that check, I should have noticed.
I am not sure if I fully understand your second point as I see that as feature/benefit. Is it going against the design methodology? Is there an alternative method for retrieving the results into memory without hitting the file system?
There was a problem hiding this comment.
So it is against our design to have the export in the sdk model functions. In this example, if you just call
df = openbb.stocks.search("Apple")
That will be in your memory locally. If you want to save to a file you can using pandas built-ins.
I might not be understanding what you wish to accomplish. When you say you want it into memory, what are you hoping to do?
There was a problem hiding this comment.
I just pushed some commits to remove that redundant flag that I added. Updated documentation as well.
The desire behind this pull request is to enable this particular function to actually return the results in dataframe format - if any are available - exactly as you said: df = openbb.stocks.search("Apple")
Right now - from my understanding - It doesn't return anything.
There was a problem hiding this comment.
Yes that is correct. You fixed that by returning the df. We just dont need to have the export/sheet name in the model functions.
There was a problem hiding this comment.
Ah now I understand! (Hopefully).
I pushed a commit that removes the export/sheet name in that function. I adjusted and ran the tests to success.
Also redid my initial manual testing.
I also saw the mypy failures, and changed the return statements to give back an empty dataframe. Similar to other functions in the same file.
…eturn a dataframe even if empty. Update tests to reflect removed function arguments. Update documentation as well.
joey-walker
changed the title
Description
A small change to allow the search command under stocks to return a dataframe in memory so that you don't have to grab the results from the file system. Added a flag to configure enabling the exporting of the results to the filesystem.
How has this been tested?
Ran the terminal to make sure the search command under stocks still works. Tested with flag both on and off.
Also tested via importing and running under the python console.
Ran the following commands:
OpenBB Terminal
/ $ stocks /stocks/ $ search NBH Companies found on term nbh ┏━━━━━━┳━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━┓ ┃ ┃ Name ┃ Country ┃ Sector ┃ Industry ┃ Exchange ┃ ┡━━━━━━╇━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━┩ │ FNHM │ FNBH Bancorp, Inc. │ United States │ Financial Services │ Banks - Regional │ PNK │ └──────┴────────────────────┴───────────────┴────────────────────┴──────────────────┴──────────┘Python