From e9175c7ad2e791c2ba83b79997d40cce7d19c859 Mon Sep 17 00:00:00 2001 From: Stefan Jansen Date: Fri, 11 Sep 2026 12:19:04 -0400 Subject: [PATCH] Say why a Yahoo download came back empty instead of blaming the symbol `_fetch_and_transform_data` read an empty frame from `yf.download` as proof the symbol does not exist. It is not proof of anything. `yf.download` reports every per-symbol failure identically: it calls `Ticker.history` without `raise_errors`, catches whatever comes back inside `_download_one`, records it on a context object local to that call, logs it, and hands back an empty frame. So ANY cause - a throttle, a Yahoo outage, a malformed response, a transient miss - was reported to the reader as `Symbol 'AAPL' not found or invalid`, sending them to debug the one thing that was correct. Rate limiting is the instance that reached CI. The public repository's `ch02-03` job failed on 2026-09-11 with that message while the log beneath it read `YFRateLimitError('Too Many Requests. Rate limited. Try after a while.')`. The class is wider than the instance. On an empty download the provider now asks again through the path `download` itself uses, with `raise_errors=True`, and reports what comes back: a throttle as RateLimitError, a refusal Yahoo named as SymbolNotFoundError carrying that reason in `details`, an empty answer as SymbolNotFoundError exactly as before. If the second ask returns data, the first result was a transient miss and the data is returned rather than discarded. It costs one request, only on a path that has already failed. Retry comes with the mapping and needs no new mechanism. `BaseProvider.fetch_ohlcv` already wraps in tenacity with `retry_if_exception(_provider_error_is_retryable)`, retryable being a NetworkError whose `retryable` is set, and RateLimitError(NetworkError) defaults it True and honours `retry_after`. A SymbolNotFoundError is not a NetworkError, so the old reporting made a 429 fatal as well as misleading. NetworkError is added to the re-raise tuple in `_fetch_and_transform_data` for the same reason: the catch-all below it converts anything unnamed into DataValidationError, which would have turned a survivable error into a fatal one two lines after it was raised. Two things that look like the obvious fix and are not, recorded in the tests so the next reader does not re-derive them. `yf.download` has no `raise_errors` parameter, though `Ticker.history` does. And `yfinance.shared._ERRORS` is declared and never written, so it reads as a channel for per-symbol errors and carries none. Measured against yfinance 1.5.2. The behaviour depended on is upstream's, so `test_ticker_history_still_takes_raise_errors` fails if that channel disappears rather than letting the probe quietly stop raising and the defect return unnoticed. Nine tests in tests/test_yahoo_empty_download_reporting.py, and four mutations each fail the cases that own them: restoring the original raise, dropping NetworkError from the re-raise tuple, probing when the download succeeded, and mapping a throttle back to the symbol error. The existing Yahoo suites pass unchanged, 70 tests in total. Co-Authored-By: Claude Opus 5 (1M context) --- src/ml4t/data/providers/yahoo.py | 82 ++++++++- tests/test_yahoo_empty_download_reporting.py | 178 +++++++++++++++++++ 2 files changed, 255 insertions(+), 5 deletions(-) create mode 100644 tests/test_yahoo_empty_download_reporting.py diff --git a/src/ml4t/data/providers/yahoo.py b/src/ml4t/data/providers/yahoo.py index b9c1aa8..56f898d 100644 --- a/src/ml4t/data/providers/yahoo.py +++ b/src/ml4t/data/providers/yahoo.py @@ -31,6 +31,7 @@ DataNotAvailableError, DataValidationError, NetworkError, + RateLimitError, SymbolNotFoundError, ) from ml4t.data.providers.base import BaseProvider @@ -174,6 +175,9 @@ def _fetch_and_transform_data( Raises: DataNotAvailableError: If no data available for the period DataValidationError: If data format is invalid + RateLimitError: If Yahoo throttled the request. Retryable, and + :meth:`BaseProvider.fetch_ohlcv` does retry it. + SymbolNotFoundError: If the symbol is unknown to Yahoo """ interval = self.FREQUENCY_MAP.get(frequency.lower(), "1d") @@ -201,10 +205,8 @@ def _fetch_and_transform_data( ) if df_pandas.empty: - raise SymbolNotFoundError( - "yahoo", - symbol, - details={"start": start, "end": end_str, "frequency": frequency}, + df_pandas = self._resolve_empty_download( + symbol, start, end_str, interval, frequency ) # Convert to Polars with symbol column @@ -221,7 +223,11 @@ def _fetch_and_transform_data( logger.info("Successfully fetched data", symbol=symbol, rows=len(df)) return df - except (DataNotAvailableError, SymbolNotFoundError): + # NetworkError covers RateLimitError and is listed by its base deliberately: the + # catch-all below converts anything unnamed into DataValidationError, which is not + # retryable, so a network error that fell through here would be turned into a fatal + # one two lines after being raised as a survivable one. + except (DataNotAvailableError, SymbolNotFoundError, NetworkError): raise except OSError as e: @@ -236,6 +242,72 @@ def _fetch_and_transform_data( details={"symbol": symbol, "error": str(e)}, ) from e + def _resolve_empty_download( + self, symbol: str, start: str, end: str, interval: str, frequency: str + ) -> pd.DataFrame: + """Return the data an empty ``yf.download`` missed, or raise what actually went wrong. + + ``yf.download`` reports every per-symbol failure identically. It calls + ``Ticker.history`` *without* ``raise_errors``, catches whatever comes back inside + ``_download_one``, records it on a context object local to that call, logs it, and + hands back an empty frame. Nothing a caller can read survives the call: + ``yfinance.shared._ERRORS`` is declared in yfinance 1.5.2 and never written, and + ``download`` has no ``raise_errors`` parameter of its own. + + So an empty frame used to become ``SymbolNotFoundError`` whatever had happened, and a + reader who had been rate-limited was told their ticker was invalid - sent to debug the + one thing that was correct. The public repository's ``ch02-03`` job failed this way on + 2026-09-11 with ``Symbol 'AAPL' not found or invalid`` while the log beneath it read + ``YFRateLimitError('Too Many Requests. Rate limited. Try after a while.')``. + + Asking again through the path ``download`` itself uses, this time with + ``raise_errors=True``, is what recovers the reason. It costs one request and only on a + path that has already failed. If that ask succeeds, the first result was a transient + miss and its data is returned rather than discarded. + + Mapping a rate limit to :class:`RateLimitError` is also what makes a 429 survivable: + :meth:`BaseProvider.fetch_ohlcv` retries a :class:`NetworkError` whose ``retryable`` is + set and honours its ``retry_after``, so no separate backoff is needed here. + + Args: + symbol: The symbol whose download came back empty + start: Start date in YYYY-MM-DD format + end: End date in YYYY-MM-DD format, already made exclusive by the caller + interval: yfinance interval string, as passed to the download + frequency: The caller's frequency name, carried into the error details + + Returns: + The pandas DataFrame the second ask returned, when it returned one + + Raises: + RateLimitError: Yahoo throttled the request. Retryable. + SymbolNotFoundError: The symbol really is unknown, or Yahoo refused it for a + reason it named. The reason is carried in ``details``. + """ + from yfinance.exceptions import YFException, YFRateLimitError + + details = {"start": start, "end": end, "frequency": frequency} + try: + recovered = yf.Ticker(symbol).history( + start=start, + end=end, + interval=interval, + auto_adjust=True, + actions=False, + raise_errors=True, + ) + except YFRateLimitError as e: + logger.warning("Yahoo rate limited the request", symbol=symbol, error=str(e)) + raise RateLimitError("yahoo") from e + except YFException as e: + raise SymbolNotFoundError("yahoo", symbol, details={**details, "reason": str(e)}) from e + + if recovered.empty: + raise SymbolNotFoundError("yahoo", symbol, details=details) + + logger.info("Empty download recovered on a second ask", symbol=symbol, rows=len(recovered)) + return recovered + def _convert_to_polars(self, df_pandas: pd.DataFrame, symbol: str) -> pl.DataFrame: """ Convert pandas DataFrame from yfinance to Polars DataFrame. diff --git a/tests/test_yahoo_empty_download_reporting.py b/tests/test_yahoo_empty_download_reporting.py new file mode 100644 index 0000000..62a888a --- /dev/null +++ b/tests/test_yahoo_empty_download_reporting.py @@ -0,0 +1,178 @@ +"""What an empty ``yf.download`` is allowed to be reported as. + +``yf.download`` hands back an empty frame for every per-symbol failure it has: it calls +``Ticker.history`` without ``raise_errors``, catches whatever comes back inside +``_download_one``, records it on a context object local to that call, logs it, and returns +nothing a caller can read. The provider used to conclude "symbol not found" from that +emptiness alone, so **any** cause was reported as an invalid symbol. Rate limiting is the +common one and the one that reached CI, but it is an instance of the class, not the class. + +Measured against yfinance 1.5.2. The behaviour these tests pin is upstream's, so +``test_ticker_history_still_takes_raise_errors`` fails loudly if the channel the fix depends +on disappears, rather than letting the probe silently stop raising. +""" + +from unittest.mock import MagicMock, patch + +import pandas as pd +import pytest +from yfinance.exceptions import YFPricesMissingError, YFRateLimitError + +from ml4t.data.core.exceptions import ( + DataValidationError, + RateLimitError, + SymbolNotFoundError, +) +from ml4t.data.providers.yahoo import YahooFinanceProvider + +FETCH = ("AAPL", "2024-01-01", "2024-02-01", "daily") + + +def _bars() -> pd.DataFrame: + """A day of flat-column OHLCV, the shape ``Ticker.history`` returns.""" + return pd.DataFrame( + {"Open": [1.0], "High": [2.0], "Low": [0.5], "Close": [1.5], "Volume": [100]}, + index=pd.DatetimeIndex(["2024-01-02"], name="Date"), + ) + + +@pytest.fixture +def provider() -> YahooFinanceProvider: + return YahooFinanceProvider() + + +def _probing(side_effect=None, return_value=None) -> MagicMock: + """Patch the second ask, the one that recovers why the download came back empty.""" + ticker = MagicMock() + ticker.history = MagicMock(side_effect=side_effect, return_value=return_value) + factory = MagicMock(return_value=ticker) + return factory + + +class TestEmptyDownloadIsDiagnosed: + def test_a_rate_limit_is_reported_as_a_rate_limit(self, provider): + """The defect this file exists for: not 'Symbol AAPL not found or invalid'.""" + factory = _probing(side_effect=YFRateLimitError()) + with ( + patch("ml4t.data.providers.yahoo.yf.download", return_value=pd.DataFrame()), + patch("ml4t.data.providers.yahoo.yf.Ticker", factory), + pytest.raises(RateLimitError) as excinfo, + ): + provider._fetch_and_transform_data(*FETCH) + + assert "Rate limit" in str(excinfo.value) + assert "not found or invalid" not in str(excinfo.value) + + def test_a_rate_limit_is_retryable_so_the_base_class_retries_it(self, provider): + """Option 2 of the issue, delivered by the mapping rather than by new machinery. + + ``BaseProvider.fetch_ohlcv`` retries a ``NetworkError`` whose ``retryable`` is set. + A ``SymbolNotFoundError`` is not a ``NetworkError``, so the old reporting made a 429 + fatal as well as misleading. + """ + factory = _probing(side_effect=YFRateLimitError()) + with ( + patch("ml4t.data.providers.yahoo.yf.download", return_value=pd.DataFrame()), + patch("ml4t.data.providers.yahoo.yf.Ticker", factory), + pytest.raises(RateLimitError) as excinfo, + ): + provider._fetch_and_transform_data(*FETCH) + + assert excinfo.value.retryable is True + + def test_a_rate_limit_survives_the_catch_all(self, provider): + """``_fetch_and_transform_data`` turns unnamed exceptions into DataValidationError. + + That conversion would undo the fix two lines after it was made: a + ``DataValidationError`` is not a ``NetworkError`` and is therefore not retried. + """ + factory = _probing(side_effect=YFRateLimitError()) + with ( + patch("ml4t.data.providers.yahoo.yf.download", return_value=pd.DataFrame()), + patch("ml4t.data.providers.yahoo.yf.Ticker", factory), + ): + with pytest.raises(RateLimitError): + provider._fetch_and_transform_data(*FETCH) + with pytest.raises(DataValidationError): + # ...while something genuinely unclassifiable still converts. + with patch( + "ml4t.data.providers.yahoo.yf.download", + side_effect=ValueError("malformed"), + ): + provider._fetch_and_transform_data(*FETCH) + + def test_a_named_yahoo_refusal_carries_its_reason(self, provider): + """Still SymbolNotFoundError, but no longer a guess: the reason is in details.""" + factory = _probing(side_effect=YFPricesMissingError("AAPL", "no price data found")) + with ( + patch("ml4t.data.providers.yahoo.yf.download", return_value=pd.DataFrame()), + patch("ml4t.data.providers.yahoo.yf.Ticker", factory), + pytest.raises(SymbolNotFoundError) as excinfo, + ): + provider._fetch_and_transform_data(*FETCH) + + assert "reason" in excinfo.value.details + + def test_a_genuinely_unknown_symbol_is_still_reported_as_one(self, provider): + """The only case the old message was right about, and it must keep working.""" + factory = _probing(return_value=pd.DataFrame()) + with ( + patch("ml4t.data.providers.yahoo.yf.download", return_value=pd.DataFrame()), + patch("ml4t.data.providers.yahoo.yf.Ticker", factory), + pytest.raises(SymbolNotFoundError) as excinfo, + ): + provider._fetch_and_transform_data(*FETCH) + + assert "not found or invalid" in str(excinfo.value) + assert "reason" not in excinfo.value.details + + def test_a_transient_empty_download_is_recovered_rather_than_raised(self, provider): + """If the second ask answers, throwing that answer away to raise would be perverse.""" + factory = _probing(return_value=_bars()) + with ( + patch("ml4t.data.providers.yahoo.yf.download", return_value=pd.DataFrame()), + patch("ml4t.data.providers.yahoo.yf.Ticker", factory), + ): + df = provider._fetch_and_transform_data(*FETCH) + + assert df.height == 1 + assert df["close"].to_list() == [1.5] + + +class TestTheHappyPathIsUntouched: + def test_a_non_empty_download_never_asks_twice(self, provider): + """The probe costs a request, so it must only run on a path that already failed.""" + factory = _probing(return_value=_bars()) + with ( + patch("ml4t.data.providers.yahoo.yf.download", return_value=_bars()), + patch("ml4t.data.providers.yahoo.yf.Ticker", factory), + ): + df = provider._fetch_and_transform_data(*FETCH) + + assert df.height == 1 + factory.assert_not_called() + + +class TestTheUpstreamChannelTheFixDependsOn: + def test_ticker_history_still_takes_raise_errors(self): + """``yf.download`` has no ``raise_errors``; the path it calls internally does. + + If a yfinance release removes it, the probe stops raising and every empty download + silently reports "symbol not found" again - the defect, back and invisible. + """ + import inspect + + import yfinance as yf + from yfinance.scrapers.history import PriceHistory + + assert "raise_errors" in inspect.signature(PriceHistory.history).parameters + assert "raise_errors" not in inspect.signature(yf.download).parameters + + def test_shared_errors_is_not_a_channel(self): + """It looks like one. In 1.5.2 it is declared and never written. + + Recorded so the next reader does not spend the time finding that out again. + """ + import yfinance.shared as shared + + assert shared._ERRORS == {}