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

Fix 'raise_errors' argument ignored in Ticker.history() #1806

Merged
merged 1 commit into from
Dec 31, 2023

Conversation

puntonim
Copy link
Contributor

I'd like to be able to handle any exception that might happen while Ticker.history() performs the HTTP request.
I currently do it in a hackish way using a monkey patch.
With this PR I'd like to introduce a hook for a custom function that, when given, it is invoked when such exception happens.

Most basic example I can think of:

import requests
import yfinance as yf
from pandas import DataFrame

def reraise(exc):
    raise exc

ticker = yf.Ticker("AAPL")

try:
    data: DataFrame = ticker.history(
        interval="1m",
        start=...,
        end=...,
        timeout=10,
        raise_errors=True,
        request_exc_hook=reraise,
    )
except requests.ConnectTimeout as exc:
    ...

Note that my actual case a bit more complex to explain and boring (meaning not really useful for this proposal).

@ValueRaider
Copy link
Collaborator

To have any chance of this being merged you're gonna need to explain your use case, because maybe there is a better way.

@puntonim
Copy link
Contributor Author

@ValueRaider thanks for the prompt reply 😄 ! I'll try to explain better.

Let's consider this:

import requests
import yfinance as yf
from pandas import DataFrame

def get_prices(symbol, timeout=(10, 10)):
    ticker = yf.Ticker(symbol)
    data: DataFrame = ticker.history(
        period="5d",
        interval="1d",
        timeout=timeout,
    )
    return data

# Network timeout (simulated by using a really short read-timeout).
prices = get_prices("AAPL", timeout=(10, 0.01))
assert prices.empty

# Non-existent symbol.
prices = get_prices("XXXAAPLXXX")
assert prices.empty

The current code swallows any exception that YfData.get() might raise, including all Requests' exception.
And it returns an empty DataFrame.
So it is impossible for the consumer code to tell if the empty DataFrame is due to a (read or connect) timeout, a DNS failure, refused connection, any other network issue at client or Yahoo-side, JSON-encoding error, etc. In all these cases an empty DataFrame is returned exactly like the "legit" case when the symbol does not exist or there is no data for the requested period/interval.
My use case is exactly this: handling network errors.

With my proposed code change this would be possible:

def reraise(exc):
    raise exc

def get_prices(symbol, timeout=(10, 10)):
    ticker = yf.Ticker(symbol)
    data: DataFrame = ticker.history(
        period="5d",
        interval="1d",
        timeout=timeout,
        request_exc_hook=reraise,
    )
    return data

# Network timeout (simulated by using a really short read-timeout).
try:
    prices = get_prices("AAPL", timeout=(10, 0.01))
except requests.Timeout:
    print("Timeout, retrying one more time...")
    ...

My proposed code change is backward compatible and the new feature is disabled by default, so no change is required by current users, unless they want to use the new feature.

I see 2 possible alternatives that do not require introducing the request_exc_hook arg:

  • expecting all "reasonable" exceptions (like network errors) and raising custom exceptions: to me the best one, but it's not the style in this repo and it would not be backward compatible.
  • do not swallow exceptions but let them bubble-up, so to let the consumer code decide if ignoring them or not: it would not be backward compatible.

@ValueRaider
Copy link
Collaborator

Seems best fix is much simpler - add raise_errors logic to that swallowed exception. Ctrl+F "raise_errors"

@puntonim
Copy link
Contributor Author

@ValueRaider done 😉!

@puntonim puntonim changed the title Add a hook invoked when Ticker.history() receives and HTTP request exc Ticker.history() to raise HTTP request excs if raise_errors args is True Dec 31, 2023
@ValueRaider
Copy link
Collaborator

Squash commits then rebase to dev, guide here #1084

@puntonim puntonim changed the base branch from main to dev December 31, 2023 13:39
@puntonim puntonim changed the base branch from dev to main December 31, 2023 13:40
@puntonim puntonim force-pushed the ticker-history-exc-hook branch from d81c25b to c94cbb6 Compare December 31, 2023 13:58
@puntonim puntonim changed the base branch from main to dev December 31, 2023 13:58
@puntonim
Copy link
Contributor Author

Squash commits then rebase to dev, guide here #1084

@ValueRaider 👍

@ValueRaider ValueRaider merged commit 7e6ad08 into ranaroussi:dev Dec 31, 2023
@ValueRaider ValueRaider mentioned this pull request Jan 6, 2024
@ValueRaider ValueRaider changed the title Ticker.history() to raise HTTP request excs if raise_errors args is True Fix 'raise_errors' argument ignored in Ticker.history() Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants