Read every ROOT string type as plain bytes with ROOTString (#68, #20)

docs/design.md

@@ -29,8 +29,8 @@ The entry point for deserialization is the `read` method, which calls
 `update_members` on all the subclasses in the inheritance tree to build up the
 dictionary of class members (`Members`). Some classes may override `read` to
 implement header parsing, or to handle layouts that are not simply in base class
-order. The `read_mupdate_membersembers` method is only responsible for reading
-the members of the class, not any base class members.
+order. The `update_members` method is only responsible for reading the members
+of the class, not any base class members.
 
 The `update_members` signature has a type alias in `serializable.py`:
 
@@ -104,13 +104,24 @@ or by `bytes`:
 ```python
 @serializable
 class TNamed(TObject):
-    fName: Annotated[bytes, TString]
-    fTitle: Annotated[bytes, TString]
+    fName: Annotated[bytes, ROOTString("TString")]
+    fTitle: Annotated[bytes, ROOTString("TString")]
 ```
 
-where the `@serializable` decorator takes care of the conversion between `str`
-and `TString`. In this library, we will prefer to use the second approach when
-feasible.
+where the `ROOTString` annotation tells the `@serializable` decorator how the
+bytes are encoded on disk. In this library, we will prefer to use the second
+approach when feasible. Every string type is read this way: `ROOTString` has one
+encoding per length format (`"TString"`, `"charstar"` and `"RNTuple"`), and
+`framed=True` when a byte count and version word come first, as for a
+`std::string` data member. `bootstrap.TString` is the alias
+`Annotated[bytes, ROOTString("TString")]` for hand-written classes.
+
+A design assumption follows: the bytes do not record which ROOT type or encoding
+they came from. A member keeps it in its annotation, a container element in the
+container's type, and a record of its own in its key's `fClassName`, so writing
+those back needs that context, not just the `bytes`. A string read through a
+pointer keeps it nowhere: the stream's class tag is read to find the type and
+then dropped, so such a value cannot be written back as read (#135).
 
 ## Data Fetching and Locators
 

src/rootfilespec/bootstrap/TDirectory.py

@@ -198,10 +198,10 @@ def __len__(self):
     # Key names are uninterpreted bytes (root-io-spec Conventions §5.1), so the
     # mapping is keyed by the bytes as stored, with no decoding.
     def __iter__(self):
-        return (key.fName.fString for key in self.fKeys)
+        return (key.fName for key in self.fKeys)
 
     def __getitem__(self, key: bytes):
-        matches = [k for k in self.fKeys if k.fName.fString == key]
+        matches = [k for k in self.fKeys if k.fName == key]
         if not matches:
             raise KeyError(key)
         return max(matches, key=lambda k: k.header.fCycle)

src/rootfilespec/bootstrap/TKey.py

@@ -11,9 +11,10 @@
     Members,
     ReadBuffer,
     ROOTSerializable,
+    read_value,
     serializable,
 )
-from rootfilespec.structutil import Fmt
+from rootfilespec.structutil import Fmt, ROOTString
 
 
 @serializable
@@ -91,9 +92,9 @@ def update_members(
             (fSeekKey, fSeekPdir), buffer = buffer.unpack(">ii")
         else:
             (fSeekKey, fSeekPdir), buffer = buffer.unpack(">qq")
-        fClassName, buffer = TString.read(buffer)
-        fName, buffer = TString.read(buffer)
-        fTitle, buffer = TString.read(buffer)
+        fClassName, buffer = ROOTString("TString").read(buffer)
+        fName, buffer = ROOTString("TString").read(buffer)
+        fTitle, buffer = ROOTString("TString").read(buffer)
         if header.fVersion % 1000 not in (2, 4):
             msg = f"TKey.read_members: unexpected version {header.fVersion}"
             raise ValueError(msg)
@@ -118,7 +119,7 @@ def read_object(
         fetch_data: DataFetcher,
         objtype: type[ObjType] | None = None,
     ) -> ObjType | ROOTSerializable:
-        if self.fClassName.fString == b"RBlob":
+        if self.fClassName == b"RBlob":
             # An RBlob key's fObjLen is decorative, and one blob can hold several
             # pages, each with a checksum fObjLen does not count (root-io-spec
             # RNTuple NOTES 1). Reading it as a key payload silently truncates it.
@@ -138,13 +139,16 @@ def read_object(
             buffer = decompress(buffer, self.header.fObjlen)
         else:
             buffer = buffer[: self.header.fObjlen]
-        if objtype is not None:
-            typename = objtype.__name__
-            obj, buffer = objtype.read(buffer)
+        readtype: type[ObjType] | type[ROOTSerializable]
+        if objtype is None:
+            typename = normalize(self.fClassName)
+            readtype = buffer.file_context.type_by_name(typename)
         else:
-            typename = normalize(self.fClassName.fString)
-            dyntype = buffer.file_context.type_by_name(typename)
-            obj, buffer = dyntype.read(buffer)  # type: ignore[assignment]
+            typename = objtype.__name__
+            readtype = objtype
+        # A looked-up type may be an annotated builtin, such as TString (#68)
+        obj: ObjType | ROOTSerializable
+        obj, buffer = read_value(readtype, buffer)
         # Some types we have to handle trailing bytes
         if typename == "TKeyList":
             # TODO: understand this padding
@@ -194,7 +198,7 @@ def size(self) -> int:
     @classmethod
     def read(cls, buffer: ReadBuffer):
         key, buffer = TKey.read(buffer)
-        typename = normalize(key.fClassName.fString)
+        typename = normalize(key.fClassName)
         objtype = buffer.file_context.type_by_name(typename)
         return cls(key=key, objtype=objtype), buffer  # type: ignore[arg-type]
 

src/rootfilespec/bootstrap/TList.py

@@ -9,7 +9,7 @@
     ROOTSerializable,
     serializable,
 )
-from rootfilespec.structutil import Fmt
+from rootfilespec.structutil import Fmt, ROOTString
 
 
 @serializable
@@ -47,9 +47,9 @@ def update_members(cls, members: Members, buffer: ReadBuffer):
                 raise ValueError(msg)
             # Each entry is followed by its option string (counted, usually empty)
             # TODO: version gates, see root-io-spec StreamerInfo.md §4 (issue #103)
-            option, buffer = TString.read(buffer)
+            option, buffer = ROOTString("TString").read(buffer)
             items.append(item)
-            options.append(option.fString)
+            options.append(option)
         members["items"] = items
         members["options"] = options
         return members, buffer

src/rootfilespec/bootstrap/TStreamerInfo.py

@@ -27,27 +27,27 @@ class TStreamerInfo(TNamed):
 
     def class_name(self) -> str:
         """Get the class name of this streamer info."""
-        return normalize(self.fName.fString)
+        return normalize(self.fName)
 
     def check_classname(self) -> None:
-        """We will use normalize(self.fName.fString) to lookup the class name during reading.
+        """We will use normalize(self.fName) to lookup the class name during reading.
 
         But other types may be templated containers of this type so we need to ensure cpptype_to_pytype
         returns the same class name.
         """
         if (
             self.fObjects.fSize == 1
             and isinstance(self.fObjects.objects[0], TStreamerSTL)
-            and self.fObjects.objects[0].fName.fString == b"This"
+            and self.fObjects.objects[0].fName == b"This"
         ):
             # TODO: understand the purpose of these intermediate member types
             return
-        if self.fName.fString.startswith(b"pair<") and self.fObjects.fSize == 2:
+        if self.fName.startswith(b"pair<") and self.fObjects.fSize == 2:
             return
         clsname = self.class_name()
-        typename, _ = cpptype_to_pytype(self.fName.fString)
+        typename, _ = cpptype_to_pytype(self.fName)
         if clsname != typename:
-            msg = f"Class name mismatch: {clsname} != {typename} (raw: {self.fName.fString!r})"
+            msg = f"Class name mismatch: {clsname} != {typename} (raw: {self.fName!r})"
             raise ValueError(msg)
 
     def base_classes(self) -> list[str]:
@@ -70,9 +70,9 @@ def base_classes(self) -> list[str]:
     def element(self, name: bytes) -> "TStreamerElement":
         """Get the streamer element of the member (or base class) with this name."""
         for element in self.fObjects.objects:
-            if isinstance(element, TStreamerElement) and element.fName.fString == name:
+            if isinstance(element, TStreamerElement) and element.fName == name:
                 return element
-        msg = f"{self.fName.fString!r} has no element named {name!r}"
+        msg = f"{self.fName!r} has no element named {name!r}"
         raise KeyError(msg)
 
     def class_definition(self) -> ClassDef:
@@ -91,7 +91,7 @@ def class_definition(self) -> ClassDef:
                 continue
             mdef, dep = element.member_definition(parent=self)
             dependencies.extend(dep)
-            mdoc = element.fTitle.fString.decode(ENCODING).strip()
+            mdoc = element.fTitle.decode(ENCODING).strip()
             # Prevent a syntax error from four consecutive double quotes
             if mdoc.endswith('"'):
                 mdoc += " "
@@ -129,8 +129,6 @@ def _structtype_to_pytype(fmt: str) -> type[int | float | bool | bytes]:
         return bool
     if fmt in ("float16", "double32"):
         return float
-    if fmt == "charstar":
-        return bytes
     msg = f"Unknown format character {fmt!r}"
     raise ValueError(msg)
 
@@ -321,11 +319,11 @@ class TStreamerElement(TNamed):
 
     def member_name(self) -> str:
         """Get the member name of this streamer element."""
-        return normalize(self.fName.fString)
+        return normalize(self.fName)
 
     def type_name(self) -> str:
         """Get the type name of this streamer element."""
-        return normalize(self.fTypeName.fString)
+        return normalize(self.fTypeName)
 
     def member_definition(self, parent: TStreamerInfo) -> tuple[str, list[str]]:
         """Get the member definition of this streamer element.
@@ -365,7 +363,7 @@ def member_definition(self, parent: TStreamerInfo):  # noqa: ARG002
 
         # In TStreamerBasicType.member_definition
         if self.fType == ElementType.kDouble32:
-            title = self.fTitle.fString.decode("utf-8", errors="replace").strip()
+            title = self.fTitle.decode("utf-8", errors="replace").strip()
             xmin, xmax, nbits, factor = parse_double32_title(title)
 
             return (
@@ -374,6 +372,9 @@ def member_definition(self, parent: TStreamerInfo):  # noqa: ARG002
             )
 
         fmt = self.fType.as_fmt()
+        if fmt == "charstar":
+            # A char* member: i32 length, then the bytes (Conventions §5.4, #20)
+            return f"{self.member_name()}: Annotated[bytes, ROOTString('charstar')]", []
         pytype = _structtype_to_pytype(fmt).__name__
         return f"{self.member_name()}: Annotated[{pytype}, Fmt({fmt!r})]", []
 
@@ -384,7 +385,7 @@ def member_definition(self, parent: TStreamerInfo):  # noqa: ARG002
         if self.fArrayLength > 0:
             msg = f"Array length not implemented for {self.__class__.__name__}"
             raise NotImplementedError(msg)
-        return f"{self.member_name()}: TString", []
+        return f"{self.member_name()}: Annotated[bytes, ROOTString('TString')]", []
 
 
 @serializable
@@ -412,12 +413,12 @@ def member_definition(self, parent: TStreamerInfo):  # noqa: ARG002
         # which requires the TStreamerInfo instances themselves to be linked
         # if not (
         #     self.fCountClass == parent.fName
-        #     or normalize(self.fCountClass.fString) in parent.base_classes()
+        #     or normalize(self.fCountClass) in parent.base_classes()
         # ):
         #     msg = f"fCountClass {self.fCountClass} != parent.fName {parent.fName}"
         #     raise ValueError(msg)
 
-        countname = normalize(self.fCountName.fString)
+        countname = normalize(self.fCountName)
         atype = f"BasicArray({fmt!r}, {countname!r})"
         return (
             f"{self.member_name()}: Annotated[np.ndarray, {atype}]",
@@ -446,7 +447,7 @@ def member_definition(self, parent: TStreamerInfo):
         if self.fArrayLength > 0:
             msg = f"Array length not implemented for {self.__class__.__name__}"
             raise NotImplementedError(msg)
-        ctype = self.fTypeName.fString
+        ctype = self.fTypeName
         if not ctype.endswith(b"*"):
             # appears to happen when std::unique_ptr<T> is used
             # e.g. RooRealVar's std::unique_ptr<RooAbsBinning> _binning (since v6-21-02)
@@ -482,9 +483,9 @@ def member_definition(self, parent: TStreamerInfo):  # noqa: ARG002
         # TODO: check fCountClass is in parent.base_classes()
         # See TStreamerBasicPointer.member_definition
 
-        countname = normalize(self.fCountName.fString)
+        countname = normalize(self.fCountName)
         atype = f"ObjectArray({countname!r})"
-        itemtype, dependencies = cpptype_to_pytype(self.fTypeName.fString)
+        itemtype, dependencies = cpptype_to_pytype(self.fTypeName)
         return (
             f"{self.member_name()}: Annotated[list[{itemtype}], {atype}]",
             list(dependencies),
@@ -499,7 +500,7 @@ def member_definition(self, parent: TStreamerInfo):
             return f"{self.member_name()}: {typename}", []
         # This may be a non-trivial type, e.g. vector<double>
         # or vector<TLorentzVector>
-        typename, dependencies = cpptype_to_pytype(self.fTypeName.fString)
+        typename, dependencies = cpptype_to_pytype(self.fTypeName)
         if self.fArrayLength > 0:
             typename = f"Annotated[list[{typename}], ObjectArray({self.fArrayLength})]"
         return f"{self.member_name()}: {typename}", list(dependencies)
@@ -511,10 +512,8 @@ def member_definition(self, parent: TStreamerInfo):
         if self.fArrayLength > 0:
             msg = f"Array length not implemented for {self.__class__.__name__}"
             raise NotImplementedError(msg)
-        # assert self.fTypeName.fString.endswith(b"*"), f"Expected pointer type, got {self.fTypeName.fString!r}"
-        typename, dependencies = cpptype_to_pytype(
-            self.fTypeName.fString.removesuffix(b"*")
-        )
+        # assert self.fTypeName.endswith(b"*"), f"Expected pointer type, got {self.fTypeName!r}"
+        typename, dependencies = cpptype_to_pytype(self.fTypeName.removesuffix(b"*"))
         if typename == parent.class_name():
             dependencies.remove(typename)
             typename = f'"{typename}"'
@@ -598,8 +597,8 @@ class TStreamerSTL(TStreamerElement):
 
     def member_definition(self, parent: TStreamerInfo):  # noqa: ARG002
         if STLType.kOffsetP <= self.fSTLtype < STLType.kOffsetP + STLType.kSTLend:
-            assert self.fTypeName.fString.endswith(b"*")
-        typename, dependencies = cpptype_to_pytype(self.fTypeName.fString)
+            assert self.fTypeName.endswith(b"*")
+        typename, dependencies = cpptype_to_pytype(self.fTypeName)
         return f"{self.member_name()}: {typename}", list(dependencies)
 
 
@@ -608,4 +607,8 @@ class TStreamerSTLstring(TStreamerSTL):
     """STL string streamer element."""
 
     def member_definition(self, parent: TStreamerInfo):  # noqa: ARG002
-        return f"{self.member_name()}: STLString", []
+        # A std::string data member has a byte count and version word (Conventions §5.3)
+        return (
+            f"{self.member_name()}: Annotated[bytes, ROOTString('TString', framed=True)]",
+            [],
+        )

src/rootfilespec/bootstrap/__init__.py

@@ -8,6 +8,7 @@
 """
 
 import dataclasses
+from typing import Any
 
 from rootfilespec.bootstrap.array import (
     TArray,
@@ -28,7 +29,7 @@
 from rootfilespec.bootstrap.double32 import Double32Serde
 from rootfilespec.bootstrap.RAnchor import ROOT3a3aRNTuple
 from rootfilespec.bootstrap.streamedobject import Ref, StreamedObject
-from rootfilespec.bootstrap.strings import RString, STLString, TString, string
+from rootfilespec.bootstrap.strings import TString, TStringLong, string
 from rootfilespec.bootstrap.TBasket import TBasket
 from rootfilespec.bootstrap.TDatime import TDatime
 from rootfilespec.bootstrap.TDirectory import TDirectory, TDirectoryFile, TKeyList
@@ -68,13 +69,15 @@ def type_by_name(
         return cls
 
 
-BOOTSTRAP_CONTEXT = _BootstrapContext(
-    types={
-        name: cls
-        for name, cls in globals().items()
-        if isinstance(cls, type) and issubclass(cls, ROOTSerializable)
-    }
-)
+_types: dict[str, Any] = {
+    name: cls
+    for name, cls in globals().items()
+    if isinstance(cls, type) and issubclass(cls, ROOTSerializable)
+}
+# A string stored as an object of its own (a key, or through a pointer) is looked
+# up by its class name, and reads as bytes (#68)
+_types |= {"TString": TString, "TStringLong": TStringLong, "string": string}
+BOOTSTRAP_CONTEXT = _BootstrapContext(types=_types)
 
 __all__ = [
     "BOOTSTRAP_CONTEXT",
@@ -83,10 +86,8 @@ def type_by_name(
     "RCompressionHeader",
     "ROOT3a3aRNTuple",
     "ROOTFile",
-    "RString",
     "Ref",
     "RooLinkedList",
-    "STLString",
     "StreamedObject",
     "TArray",
     "TArrayC",
@@ -124,6 +125,7 @@ def type_by_name(
     "TStreamerSTLstring",
     "TStreamerString",
     "TString",
+    "TStringLong",
     "TVirtualIndex",
     "Uninterpreted",
     "string",