fix(data): make FileInstrumentStorage round-trip instrument files correctly

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={})

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}"