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

Improve performance of fetching Ticker timezone #1112

Merged
merged 5 commits into from Oct 25, 2022
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 25 additions & 6 deletions yfinance/base.py
Expand Up @@ -159,7 +159,7 @@ def history(self, period="1mo", interval="1d",

if start or period is None or period.lower() == "max":
# Check can get TZ. Fail => probably delisted
tz = self._get_ticker_tz()
tz = self._get_ticker_tz(proxy, timeout)
if tz is None:
# Every valid ticker has a timezone. Missing = problem
err_msg = "No timezone found, symbol certainly delisted"
Expand Down Expand Up @@ -533,7 +533,7 @@ def _fix_unit_mixups(self, df, interval, tz_exchange):
return df


def _get_ticker_tz(self):
def _get_ticker_tz(self, proxy=None, timeout=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is _get_ticker_tz called without proxy and timeout in any other place?
As proxy and timeout is passed to the history method and not Ticker constructor, any calls to this from another methods might not have the correct proxy. (However I do not fully understand the use of proxies in yfinance so this might not be an issue).
Also if timeout is None the request will hang indefinitely, so maybe default to something reasonable.
https://requests.readthedocs.io/en/latest/user/quickstart/#timeouts
Maybe make sure any new code do not call it without setting proxy and timeout so do not make them optional. And if timeout is set to None use a default timeout .

if not self._tz is None:
return self._tz

Expand All @@ -545,10 +545,7 @@ def _get_ticker_tz(self):
tz = None

if tz is None:
try:
tz = self.info["exchangeTimezoneName"]
except KeyError:
return None
tz = self._fetch_ticker_tz(proxy, timeout)

if utils.is_valid_timezone(tz):
# info fetch is relatively slow so cache timezone
Expand All @@ -559,6 +556,28 @@ def _get_ticker_tz(self):
self._tz = tz
return tz

def _fetch_ticker_tz(self, proxy=None, timeout=None):
# Query Yahoo for basic price data just to get returned timezone

params = {"range":"1wk", "interval":"1d"}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think range can be "1d" as well to get even less data.


# setup proxy in requests format
if proxy is not None:
if isinstance(proxy, dict) and "https" in proxy:
proxy = proxy["https"]
proxy = {"https": proxy}

# Getting data from json
url = "{}/v8/finance/chart/{}".format(self._base_url, self.ticker)

session = self.session or _requests
try:
data = session.get(url=url, params=params, proxies=proxy, headers=utils.user_agent_headers, timeout=timeout)
data = data.json()
return data["chart"]["result"][0]["meta"]["exchangeTimezoneName"]
except:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe if debug is True print the exception
example:

except Exception as e:
    if debug_mode:
        print("Failed to get ticker '{}' reason: {}".format(self.ticker, e))

return None


def _get_info(self, proxy=None):
# setup proxy in requests format
Expand Down