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.

Sign up for GitHub

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

Return a dataframe from stocks search, removed export to file system (#3923) #4193

Merged
merged 8 commits into from
Feb 10, 2023
Merged

Return a dataframe from stocks search, removed export to file system (#3923) #4193

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
373f387
Return a dataframe from stocks search, allow override to not export t…
joey-walker Feb 9, 2023
06b9196
Fix documentation to reflect changes in search method in sdk
joey-walker Feb 9, 2023
f247b6d
Remove export_to_file flag for redundancy
joey-walker Feb 10, 2023
d0d1eb8
Remove erroneous extra line from documentation
joey-walker Feb 10, 2023
5c2cbf2
Merge branch 'develop' into feature/search-return-df
jmaslek Feb 10, 2023
e31c206
Merge branch 'develop' into feature/search-return-df
jmaslek Feb 10, 2023
a11810a
Remove export to file now that we are returning a dataframe. Always …
joey-walker Feb 10, 2023
047ee64
Merge branch 'develop' into feature/search-return-df
jmaslek Feb 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 25 additions & 20 deletions openbb_terminal/stocks/stocks_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ def search(
limit: int = 0,
export: str = "",
sheet_name: Optional[str] = "",
) -> None:
export_to_file: bool = True,
) -> Optional[pd.DataFrame]:
"""Search selected query for tickers.

Parameters
Expand All @@ -127,6 +128,13 @@ def search(
The limit of companies shown.
export : str
Export data
export_to_file : bool
Whether results should be exported to file or not.

Returns
-------
df: Optional[pd.DataFrame]
Dataframe of search results if any results are found

Examples
--------
Expand Down Expand Up @@ -183,18 +191,13 @@ def search(
df = pd.DataFrame.from_dict(d).T[
["long_name", "short_name", "country", "sector", "industry", "exchange"]
]
if exchange_country:
if exchange_country in market_coverage_suffix:
suffix_tickers = [
ticker.split(".")[1] if "." in ticker else ""
for ticker in list(df.index)
]
df = df[
[
val in market_coverage_suffix[exchange_country]
for val in suffix_tickers
]
]
if exchange_country and exchange_country in market_coverage_suffix:
suffix_tickers = [
ticker.split(".")[1] if "." in ticker else "" for ticker in list(df.index)
]
df = df[
[val in market_coverage_suffix[exchange_country] for val in suffix_tickers]
]

exchange_suffix = {}
for k, v in market_coverage_suffix.items():
Expand Down Expand Up @@ -227,14 +230,16 @@ def search(
headers=["Name", "Country", "Sector", "Industry", "Exchange"],
title=title,
)
if export_to_file:
export_data(
export,
os.path.dirname(os.path.abspath(__file__)),
"search",
df,
sheet_name,
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


export_data(
export,
os.path.dirname(os.path.abspath(__file__)),
"search",
df,
sheet_name,
)
return df


def load(
Expand Down
29 changes: 17 additions & 12 deletions website/content/sdk/reference/stocks/search.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,34 @@ Search selected query for tickers.
Source Code: [[link](https://github.com/OpenBB-finance/OpenBBTerminal/tree/main/openbb_terminal/stocks/stocks_helper.py#L98)]

```python
openbb.stocks.search(query: str = "", country: str = "", sector: str = "", industry: str = "", exchange_country: str = "", limit: int = 0, export: str = "")
openbb.stocks.search(query: str = "", country: str = "", sector: str = "", industry: str = "", exchange_country: str = "", limit: int = 0, export: str = "", sheet_name: Optional[str] = "", export_to_file: bool = True)
```

---

## Parameters

| Name | Type | Description | Default | Optional |
| ---- | ---- | ----------- | ------- | -------- |
| query | str | The search term used to find company tickers | | True |
| country | str | Search by country to find stocks matching the criteria | | True |
| sector | str | Search by sector to find stocks matching the criteria | | True |
| industry | str | Search by industry to find stocks matching the criteria | | True |
| exchange_country | str | Search by exchange country to find stock matching | | True |
| limit | int | The limit of companies shown. | 0 | True |
| export | str | Export data | | True |

| Name | Type | Description | Default | Optional |
|------------------|------|---------------------------------------------------------|---------|----------|
| query | str | The search term used to find company tickers | | True |
| country | str | Search by country to find stocks matching the criteria | | True |
| sector | str | Search by sector to find stocks matching the criteria | | True |
| industry | str | Search by industry to find stocks matching the criteria | | True |
| exchange_country | str | Search by exchange country to find stock matching | | True |
| limit | int | The limit of companies shown. | 0 | True |
| export | str | Export data | | True |
| sheet_name | str | Sheet Name | | True |
| export_to_file | bool | Whether results should be exported to file or not. | | True |

---

## Returns

This function does not return anything
| Type | Description |
|------------------------|---------------------------------|
| Optional[pd.DataFrame] | Search results if any are found |
---


---

Expand Down