From b9117a19f2041e40f8df2f21315c8c52253e6471 Mon Sep 17 00:00:00 2001 From: Vignesh Nagarajan Date: Fri, 17 Jul 2026 19:12:01 -0700 Subject: [PATCH] fix(data): make FileInstrumentStorage round-trip instrument files correctly Two long-standing defects in FileInstrumentStorage broke the instrument file round-trip: - _read_instrument let pandas NA-parse the symbol column, so real tickers literally named "NA" or "NULL" were loaded as float NaN and merged into a single bogus instrument, which then leaked out of D.list_instruments. PR #1736 fixed this same defect in exists_qlib_data (reported in #1720), but the storage reader - the path that actually feeds data loading - kept the old behavior. - _write_instrument wrote the file twice back-to-back; the second to_csv immediately overwrote the correctly-ordered output with columns in [start, end, symbol] order, which _read_instrument then misparsed (dates became symbols). Present since the original implementation (70fc5810); every public mutation (update, __setitem__, __delitem__) went through it. Fix: disable default NA parsing when reading the symbol column (symbols are opaque identifiers), and write the file once in the documented [symbol, start, end] order, keeping the utf-8 encoding of the removed call. Both defects are covered by new self-contained tests in tests/storage_tests/test_storage.py (temporary provider, no market data), each verified to fail against the previous code. Related: #1720, #1736 Co-Authored-By: Claude Fable 5 --- qlib/data/storage/file_storage.py | 8 ++- tests/storage_tests/test_storage.py | 76 +++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/qlib/data/storage/file_storage.py b/qlib/data/storage/file_storage.py index e2bc5c3679a..c3d21b67dc5 100644 --- a/qlib/data/storage/file_storage.py +++ b/qlib/data/storage/file_storage.py @@ -212,6 +212,9 @@ def _read_instrument(self) -> Dict[InstKT, InstVT]: names=[self.SYMBOL_FIELD_NAME, self.INSTRUMENT_START_FIELD, self.INSTRUMENT_END_FIELD], dtype={self.SYMBOL_FIELD_NAME: str}, parse_dates=[self.INSTRUMENT_START_FIELD, self.INSTRUMENT_END_FIELD], + # Symbols are opaque identifiers: tickers literally named "NA" or "NULL" must not be + # read as missing values (#1736 fixed the same defect in `exists_qlib_data` only). + keep_default_na=False, ) for row in df.itertuples(index=False): _instruments.setdefault(row[0], []).append((row[1], row[2])) @@ -230,10 +233,11 @@ def _write_instrument(self, data: Dict[InstKT, InstVT] = None) -> None: res.append(_df) df = pd.concat(res, sort=False) + # A single write in the on-disk column order expected by `_read_instrument`: [symbol, start, end]. + # (Previously a second, unordered `to_csv` overwrote this file with [start, end, symbol].) df.loc[:, [self.SYMBOL_FIELD_NAME, self.INSTRUMENT_START_FIELD, self.INSTRUMENT_END_FIELD]].to_csv( - self.uri, header=False, sep=self.INSTRUMENT_SEP, index=False + self.uri, header=False, sep=self.INSTRUMENT_SEP, index=False, encoding="utf-8" ) - df.to_csv(self.uri, sep="\t", encoding="utf-8", header=False, index=False) def clear(self) -> None: self._write_instrument(data={}) diff --git a/tests/storage_tests/test_storage.py b/tests/storage_tests/test_storage.py index 92fed34ecda..51640b912ee 100644 --- a/tests/storage_tests/test_storage.py +++ b/tests/storage_tests/test_storage.py @@ -2,10 +2,15 @@ # Licensed under the MIT License. +import tempfile +import unittest from pathlib import Path from collections.abc import Iterable import numpy as np +import pandas as pd + +import qlib from qlib.tests import TestAutoData from qlib.data.storage.file_storage import ( @@ -168,3 +173,74 @@ def test_feature_storage(self): print(feature[:].empty) with self.assertRaises(ValueError): print(feature.data.empty) + + +class TestInstrumentStorageRoundTrip(unittest.TestCase): + """Self-contained InstrumentStorage read/write checks (uses a temporary provider, no market data).""" + + @classmethod + def setUpClass(cls) -> None: + # qlib.init is only needed to populate the global config defaults (e.g. `C.mount_path`) + # consulted by the storage path manager; the storages below use their own provider_uri. + cls._init_tmp = tempfile.TemporaryDirectory() + init_path = Path(cls._init_tmp.name) + init_path.joinpath("calendars").mkdir() + init_path.joinpath("calendars", "day.txt").write_text("2020-01-02\n") + init_path.joinpath("instruments").mkdir() + init_path.joinpath("features").mkdir() + qlib.init(provider_uri=cls._init_tmp.name, expression_cache=None, dataset_cache=None) + + @classmethod + def tearDownClass(cls) -> None: + cls._init_tmp.cleanup() + + def setUp(self) -> None: + self._tmp_dir = tempfile.TemporaryDirectory() + provider_path = Path(self._tmp_dir.name) + # the supported frequencies of a provider are derived from the calendar file names + provider_path.joinpath("calendars").mkdir() + provider_path.joinpath("calendars", "day.txt").write_text("2020-01-02\n2020-01-03\n2020-01-06\n") + provider_path.joinpath("instruments").mkdir() + self.provider_path = provider_path + + def tearDown(self) -> None: + self._tmp_dir.cleanup() + + def _write_market_file(self, market: str, content: str) -> None: + self.provider_path.joinpath("instruments", f"{market}.txt").write_text(content) + + def test_symbols_named_like_na_sentinels(self): + """ + Tickers literally named "NA" or "NULL" must be loaded as strings. + + Reading them as NaN merges distinct tickers into one bogus float key (see issue #1720, + whose fix in #1736 covered exists_qlib_data but not this storage reader). + """ + self._write_market_file( + "na_market", + "NA\t2020-01-02\t2020-01-06\nNULL\t2020-01-02\t2020-01-06\nSH600000\t2020-01-02\t2020-01-06\n", + ) + instrument = InstrumentStorage(market="na_market", freq="day", provider_uri=self._tmp_dir.name) + data = instrument.data + + assert set(data.keys()) == {"NA", "NULL", "SH600000"}, f"symbols were NA-parsed: {list(data.keys())}" + for symbol, spans in data.items(): + assert isinstance(symbol, str), f"symbol {symbol!r} is not str" + assert spans == [ + (pd.Timestamp("2020-01-02"), pd.Timestamp("2020-01-06")) + ], f"wrong spans for {symbol!r}: {spans}" + + def test_write_read_round_trip(self): + """ + Instruments written through the public mutation API must read back unchanged + (`_read_instrument` expects the on-disk column order [symbol, start, end]). + """ + self._write_market_file("rt_market", "SH600000\t2020-01-02\t2020-01-06\n") + instrument = InstrumentStorage(market="rt_market", freq="day", provider_uri=self._tmp_dir.name) + instrument["SH600001"] = [(pd.Timestamp("2020-01-03"), pd.Timestamp("2020-01-06"))] + + reread = InstrumentStorage(market="rt_market", freq="day", provider_uri=self._tmp_dir.name).data + assert reread == { + "SH600000": [(pd.Timestamp("2020-01-02"), pd.Timestamp("2020-01-06"))], + "SH600001": [(pd.Timestamp("2020-01-03"), pd.Timestamp("2020-01-06"))], + }, f"round-trip corrupted the instrument file: {reread}"