From 41244f8e84ec667a70cd444cacc3eede6b97bc4a Mon Sep 17 00:00:00 2001 From: Samantha Abbott <52800387+sathabbott@users.noreply.github.com> Date: Thu, 24 Sep 2026 17:11:01 +0000 Subject: [PATCH 1/9] Read RNTuple strings as plain bytes docs/design.md promises that where a builtin Python type captures a ROOT type, the deserialized member is that builtin: bytes for strings. RNTuple strings were RString wrappers instead. Add CountedString, a MemberSerDe for a string stored as its length (in a given struct format) followed by that many bytes, read as plain bytes. An RNTuple string is Annotated[bytes, CountedString("i"). Part of #68 Assisted-by: claude-code:claude-opus-5-5 --- src/rootfilespec/bootstrap/__init__.py | 3 +- src/rootfilespec/bootstrap/strings.py | 23 ------- src/rootfilespec/rntuple/header.py | 8 +-- src/rootfilespec/rntuple/schema.py | 19 +++--- src/rootfilespec/structutil.py | 28 +++++++++ tests/test_rntuple_hardcoded.py | 83 +++++++++++++------------- tests/test_rntuple_strings.py | 68 +++++++++++++++++++++ 7 files changed, 154 insertions(+), 78 deletions(-) create mode 100644 tests/test_rntuple_strings.py diff --git a/src/rootfilespec/bootstrap/__init__.py b/src/rootfilespec/bootstrap/__init__.py index 95e6132..20d9a3e 100644 --- a/src/rootfilespec/bootstrap/__init__.py +++ b/src/rootfilespec/bootstrap/__init__.py @@ -28,7 +28,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 STLString, TString, string from rootfilespec.bootstrap.TBasket import TBasket from rootfilespec.bootstrap.TDatime import TDatime from rootfilespec.bootstrap.TDirectory import TDirectory, TDirectoryFile, TKeyList @@ -83,7 +83,6 @@ def type_by_name( "RCompressionHeader", "ROOT3a3aRNTuple", "ROOTFile", - "RString", "Ref", "RooLinkedList", "STLString", diff --git a/src/rootfilespec/bootstrap/strings.py b/src/rootfilespec/bootstrap/strings.py index 3f2e618..bf34860 100644 --- a/src/rootfilespec/bootstrap/strings.py +++ b/src/rootfilespec/bootstrap/strings.py @@ -54,26 +54,3 @@ def update_members(cls, members: Members, buffer: ReadBuffer): data, buffer = buffer.consume(length) members["value"] = data return members, buffer - - -@serializable -class RString(ROOTSerializable): - """A class representing an RString.""" - - fString: bytes - """The string data.""" - - def __hash__(self) -> int: - return hash(self.fString) - - @classmethod - def update_members(cls, members: Members, buffer: ReadBuffer): - """Reads an RString from the given buffer. - RStrings are always prefixed with a 32bit unsigned integer indicating the length of the string. - String data are UTF-8 encoded. - """ - - (length,), buffer = buffer.unpack(" tuple[Members, ReadBuffer]: + (length,), buffer = buffer.unpack(self.length_fmt) + members[self.fname], buffer = buffer.consume(length) + return members, buffer + + +@dataclasses.dataclass +class CountedString(MemberSerDe): + """A string stored as its length, then that many bytes, read as plain ``bytes`` + + ``length_fmt`` is the struct format of the length, e.g. ``" ReadBuffer: + return ReadBuffer(memoryview(data), 0, BOOTSTRAP_CONTEXT, BufferContext(abspos=0)) + + +def test_counted_string_is_plain_bytes(): + """Issue #68: a counted string is read as the bytes on disk, not decoded""" + name = "héllo·wörld".encode() + data = len(name).to_bytes(4, "little") + name + b"\x00\x00\x00\x00" + b"rest" + obj, rest = _Named.read(_buffer(data)) + assert obj.name == name + assert type(obj.name) is bytes + assert obj.after == b"" + assert bytes(rest.data) == b"rest" + + +def test_counted_string_too_short(): + with pytest.raises(Exception): # noqa: B017, PT011 + _Named.read(_buffer(b"\x09\x00\x00\x00abc")) + + +@pytest.mark.skipif(not DATA.exists(), reason="reference/root-io-spec not checked out") +def test_rntuple_strings_are_bytes(): + """Every RNTuple string member of every root-io-spec RNTuple fixture is bytes""" + for path in sorted(DATA.glob("*.root")): + with open_path(path) as reader: + keylist = reader.keylist() + for name in keylist: + key = keylist[name] + if key.fClassName.fString != b"ROOT::RNTuple": + continue + rntuple = RNTuple.from_anchor(reader.fetch(key), reader.fetch.buffer) + header = rntuple.headerEnvelope + assert type(header.fName) is bytes + assert header.fName == name.encode() + assert type(header.fLibrary) is bytes + for field in rntuple.schemaDescription.fieldDescriptions: + for value in ( + field.fFieldName, + field.fTypeName, + field.fTypeAlias, + field.fFieldDescription, + ): + assert type(value) is bytes From 16a3ae817136206f556dbe24df3691014aa18055 Mon Sep 17 00:00:00 2001 From: Samantha Abbott <52800387+sathabbott@users.noreply.github.com> Date: Wed, 30 Sep 2026 14:18:15 +0000 Subject: [PATCH 2/9] Rename tests/test_rntuple_strings.py to tests/test_counted_string.py Name the module after what it tests, so it does not read as the home of a broad set of tests (review on #125). Assisted-by: claude-code:claude-opus-5-5 --- tests/{test_rntuple_strings.py => test_counted_string.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{test_rntuple_strings.py => test_counted_string.py} (100%) diff --git a/tests/test_rntuple_strings.py b/tests/test_counted_string.py similarity index 100% rename from tests/test_rntuple_strings.py rename to tests/test_counted_string.py From 215f8a2de28fb34b6d44235fdd863df28b284afa Mon Sep 17 00:00:00 2001 From: Samantha Abbott <52800387+sathabbott@users.noreply.github.com> Date: Wed, 30 Sep 2026 20:40:04 +0000 Subject: [PATCH 3/9] Compare header names with the key names as bytes, after #130 Assisted-by: claude-code:claude-opus-5-5 --- tests/test_counted_string.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_counted_string.py b/tests/test_counted_string.py index ef27460..b390cbb 100644 --- a/tests/test_counted_string.py +++ b/tests/test_counted_string.py @@ -56,7 +56,7 @@ def test_rntuple_strings_are_bytes(): rntuple = RNTuple.from_anchor(reader.fetch(key), reader.fetch.buffer) header = rntuple.headerEnvelope assert type(header.fName) is bytes - assert header.fName == name.encode() + assert header.fName == name assert type(header.fLibrary) is bytes for field in rntuple.schemaDescription.fieldDescriptions: for value in ( From f9d183606c1b1933be7596dc7e20db410eded2bb Mon Sep 17 00:00:00 2001 From: Samantha Abbott <52800387+sathabbott@users.noreply.github.com> Date: Wed, 30 Sep 2026 20:51:36 +0000 Subject: [PATCH 4/9] Read every ROOT string type as plain bytes with ROOTString (rest of #68, #20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace CountedString with ROOTString(encoding), one Literal value per on-disk form (root-io-spec Conventions §5), as suggested in review: - "RNTuple": u32 little-endian length, then the bytes - "TString": the counted string, one length byte or 255 then a u32 - "std::string": a std::string data member, the counted string inside a byte count and version word (§5.3); the byte count is checked - "charstar": a char* member, i32 length then the bytes, no 255 escape; a length <= 0 reads as b"" (§5.4, #20) TString is now the alias Annotated[bytes, ROOTString("TString")], so every TString member is bytes and .fString is gone. STLString is removed. read_string(buffer, encoding) reads one string by hand (TKey, TList options). Generated classes write the annotation out: TStreamerString emits ROOTString('TString'), TStreamerSTLstring ROOTString('std::string'), a kCharStar basic type ROOTString('charstar'), and cpptype maps string, std::string and TString elements of containers and pointees to ROOTString('TString'). A string stored as an object of its own (a key, or through a pointer) is looked up by class name, so TString, TStringLong and string stay resolvable by name, and serializable.read_value reads either a class or an annotated builtin. TStringLong is the "charstar" encoding (§5.1.1). As a result, root-io-spec's serialization/stringlong.root now reads, and serialization/unframed-records.root reads its TDatime, TString and TStringLong records and stops at the TObject record (#123). Assisted-by: claude-code:claude-opus-5-5 --- docs/design.md | 13 +- src/rootfilespec/bootstrap/TDirectory.py | 4 +- src/rootfilespec/bootstrap/TKey.py | 17 +- src/rootfilespec/bootstrap/TList.py | 6 +- src/rootfilespec/bootstrap/TStreamerInfo.py | 56 ++--- src/rootfilespec/bootstrap/__init__.py | 21 +- src/rootfilespec/bootstrap/streamedobject.py | 6 +- src/rootfilespec/bootstrap/strings.py | 67 ++---- src/rootfilespec/cpptype.py | 13 ++ src/rootfilespec/dynamic.py | 2 +- src/rootfilespec/generated/__init__.py | 2 +- src/rootfilespec/reader.py | 6 +- src/rootfilespec/rntuple/schema.py | 4 +- src/rootfilespec/serializable.py | 12 + src/rootfilespec/structutil.py | 74 ++++++- tests/test_counted_string.py | 68 ------ tests/test_key_compression_and_names.py | 6 +- tests/test_read.py | 14 +- tests/test_root_string.py | 220 +++++++++++++++++++ tests/test_spec_cases.py | 4 +- tests/test_spec_fixtures.py | 13 +- tests/test_tdatime.py | 19 ++ 22 files changed, 434 insertions(+), 213 deletions(-) delete mode 100644 tests/test_counted_string.py create mode 100644 tests/test_root_string.py diff --git a/docs/design.md b/docs/design.md index 3b8774c..519f21b 100644 --- a/docs/design.md +++ b/docs/design.md @@ -104,13 +104,16 @@ 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 on-disk form (`"TString"`, `"std::string"`, `"charstar"` and +`"RNTuple"`), and `bootstrap.TString` is the alias +`Annotated[bytes, ROOTString("TString")]` for hand-written classes. ## Data Fetching and Locators diff --git a/src/rootfilespec/bootstrap/TDirectory.py b/src/rootfilespec/bootstrap/TDirectory.py index 2665bb1..6dea177 100644 --- a/src/rootfilespec/bootstrap/TDirectory.py +++ b/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) diff --git a/src/rootfilespec/bootstrap/TKey.py b/src/rootfilespec/bootstrap/TKey.py index 4e4423c..5e228eb 100644 --- a/src/rootfilespec/bootstrap/TKey.py +++ b/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, read_string @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 = read_string(buffer, "TString") + fName, buffer = read_string(buffer, "TString") + fTitle, buffer = read_string(buffer, "TString") 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. @@ -142,9 +143,9 @@ def read_object( typename = objtype.__name__ obj, buffer = objtype.read(buffer) else: - typename = normalize(self.fClassName.fString) + typename = normalize(self.fClassName) dyntype = buffer.file_context.type_by_name(typename) - obj, buffer = dyntype.read(buffer) # type: ignore[assignment] + obj, buffer = read_value(dyntype, buffer) # Some types we have to handle trailing bytes if typename == "TKeyList": # TODO: understand this padding @@ -194,7 +195,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] diff --git a/src/rootfilespec/bootstrap/TList.py b/src/rootfilespec/bootstrap/TList.py index a96001a..7da73e7 100644 --- a/src/rootfilespec/bootstrap/TList.py +++ b/src/rootfilespec/bootstrap/TList.py @@ -9,7 +9,7 @@ ROOTSerializable, serializable, ) -from rootfilespec.structutil import Fmt +from rootfilespec.structutil import Fmt, read_string @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 = read_string(buffer, "TString") items.append(item) - options.append(option.fString) + options.append(option) members["items"] = items members["options"] = options return members, buffer diff --git a/src/rootfilespec/bootstrap/TStreamerInfo.py b/src/rootfilespec/bootstrap/TStreamerInfo.py index faa68d0..bf5a7b2 100644 --- a/src/rootfilespec/bootstrap/TStreamerInfo.py +++ b/src/rootfilespec/bootstrap/TStreamerInfo.py @@ -27,10 +27,10 @@ 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. @@ -38,16 +38,16 @@ def check_classname(self) -> None: 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 is used # e.g. RooRealVar's std::unique_ptr _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 # or vector - 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,5 @@ 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('std::string')]", [] diff --git a/src/rootfilespec/bootstrap/__init__.py b/src/rootfilespec/bootstrap/__init__.py index 20d9a3e..c4ff193 100644 --- a/src/rootfilespec/bootstrap/__init__.py +++ b/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 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", @@ -85,7 +88,6 @@ def type_by_name( "ROOTFile", "Ref", "RooLinkedList", - "STLString", "StreamedObject", "TArray", "TArrayC", @@ -123,6 +125,7 @@ def type_by_name( "TStreamerSTLstring", "TStreamerString", "TString", + "TStringLong", "TVirtualIndex", "Uninterpreted", "string", diff --git a/src/rootfilespec/bootstrap/streamedobject.py b/src/rootfilespec/bootstrap/streamedobject.py index 1d61ee2..3fdedbc 100644 --- a/src/rootfilespec/bootstrap/streamedobject.py +++ b/src/rootfilespec/bootstrap/streamedobject.py @@ -11,6 +11,7 @@ ReadObjMethod, ROOTSerializable, _ReadWrapper, + read_value, serializable, ) @@ -169,7 +170,10 @@ def read_streamed_item( return Ref(Some()), buffer if itemheader.fClassName: clsname = normalize(itemheader.fClassName) - dynmethod = buffer.file_context.type_by_name(clsname).read + dyntype = buffer.file_context.type_by_name(clsname) + + def dynmethod(buffer: ReadBuffer) -> tuple[ROOTSerializable, ReadBuffer]: + return read_value(dyntype, buffer) elif method is not None: clsname = f"Ref ({method})" if ( diff --git a/src/rootfilespec/bootstrap/strings.py b/src/rootfilespec/bootstrap/strings.py index bf34860..5542462 100644 --- a/src/rootfilespec/bootstrap/strings.py +++ b/src/rootfilespec/bootstrap/strings.py @@ -1,56 +1,27 @@ -from rootfilespec.bootstrap.streamedobject import StreamedObject -from rootfilespec.serializable import ( - Members, - ReadBuffer, - ROOTSerializable, - serializable, -) +from typing import Annotated +from rootfilespec.structutil import ROOTString -@serializable -class TString(ROOTSerializable): - """A class representing a TString. +TString = Annotated[bytes, ROOTString("TString")] +"""A ``TString`` data member, read as plain ``bytes`` - TODO: this can just be Annotated[bytes, TString] since nobody subclasses it - """ +A counted string: one length byte, or 255 then a 4-byte length, then the bytes +(root-io-spec Conventions §5.1). ``TString`` has no Python class: a member is +``bytes``, and the encoding lives in the annotation. Read a bare one by hand +with ``read_string(buffer, "TString")``. +""" - fString: bytes - """The string data.""" - - def __hash__(self) -> int: - return hash(self.fString) - - @classmethod - def update_members(cls, members: Members, buffer: ReadBuffer): - """Reads a TString from the given buffer. - TStrings are always prefixed with a byte indicating the length of the string. - If that byte is larger than 255, then there are 4 additional bytes are used to store the length. - - In ROOT, this is implemented at TBufferFile::ReadTString() - https://root.cern/doc/v636/TBufferFile_8cxx_source.html#l00187 - """ - (length,), buffer = buffer.unpack(">B") - if length == 255: - (length,), buffer = buffer.unpack(">i") - data, buffer = buffer.consume(length) - members["fString"] = data - return members, buffer +TStringLong = Annotated[bytes, ROOTString("charstar")] +"""A ``TStringLong``, read as plain ``bytes`` +``TStringLong`` derives from ``TString`` but writes an i32 length, then the bytes, +with no 255 escape and no frame, wherever it appears: the ``char*`` encoding +(root-io-spec Conventions §5.1.1). +""" string = TString +"""A ``std::string`` looked up by name, as an object of its own (a key's class) - -@serializable -class STLString(StreamedObject): - """String with a stream header (see also TObjString)""" - - value: bytes - - @classmethod - def update_members(cls, members: Members, buffer: ReadBuffer): - (length,), buffer = buffer.unpack(">B") - if length == 255: - (length,), buffer = buffer.unpack(">i") - data, buffer = buffer.consume(length) - members["value"] = data - return members, buffer +It is the bare counted string, as a ``TString`` is. A ``std::string`` data member +has a byte count and version word first: ``Annotated[bytes, ROOTString("std::string")]``. +""" diff --git a/src/rootfilespec/cpptype.py b/src/rootfilespec/cpptype.py index a45afa6..8d9e179 100644 --- a/src/rootfilespec/cpptype.py +++ b/src/rootfilespec/cpptype.py @@ -44,6 +44,17 @@ b"float": "Annotated[float, Fmt('>f')]", b"double": "Annotated[float, Fmt('>d')]", } + +# Strings are plain bytes, with the encoding in the annotation (#68). Here they +# are container elements or pointees, which have no byte count and version word +# of their own, so a std::string is the bare counted string, like a TString +# (root-io-spec Conventions §5.1). A std::string data member is framed: see +# TStreamerSTLstring. +_cpp_strings = { + b"TString": "Annotated[bytes, ROOTString('TString')]", + b"string": "Annotated[bytes, ROOTString('TString')]", + b"std::string": "Annotated[bytes, ROOTString('TString')]", +} # cppname -> python name, expected number of template arguments _cpp_templates: dict[bytes, tuple[str, int]] = { b"vector": ("StdVector", 1), @@ -135,6 +146,8 @@ def to_pytype(self) -> NameWithDependencies: if self.name in _cpp_primitives: deps: set[str] = set() return _cpp_primitives[self.name], deps + if self.name in _cpp_strings: + return _cpp_strings[self.name], set() pyname = normalize(self.name) return pyname, {pyname} diff --git a/src/rootfilespec/dynamic.py b/src/rootfilespec/dynamic.py index 66f0d34..bcef260 100644 --- a/src/rootfilespec/dynamic.py +++ b/src/rootfilespec/dynamic.py @@ -23,7 +23,7 @@ def streamerinfo_to_classes(streamerinfo: bootstrap.TList) -> str: for item in streamerinfo.items: if not isinstance(item, TStreamerInfo): continue - clsname = normalize(item.fName.fString) + clsname = normalize(item.fName) if clsname in declared: lines.append(f"# Class {clsname} already declared, skipping") lines.append(f"# Definition: {item}\n") diff --git a/src/rootfilespec/generated/__init__.py b/src/rootfilespec/generated/__init__.py index 7cae29d..a1347cb 100644 --- a/src/rootfilespec/generated/__init__.py +++ b/src/rootfilespec/generated/__init__.py @@ -14,4 +14,4 @@ StdVector, ) from rootfilespec.serializable import ROOTSerializable, serializable -from rootfilespec.structutil import Fmt, StdBitset +from rootfilespec.structutil import Fmt, ROOTString, StdBitset diff --git a/src/rootfilespec/reader.py b/src/rootfilespec/reader.py index e26a730..88c9e64 100644 --- a/src/rootfilespec/reader.py +++ b/src/rootfilespec/reader.py @@ -122,11 +122,7 @@ def keylist(self, directory: TDirectory | None = None) -> TKeyList: def streamerinfos(self) -> dict[bytes, TStreamerInfo]: """The TStreamerInfo of each class in the StreamerInfo record, by class name""" items = self.streamerinfo.items if self.streamerinfo else [] - return { - item.fName.fString: item - for item in items - if isinstance(item, TStreamerInfo) - } + return {item.fName: item for item in items if isinstance(item, TStreamerInfo)} def close(self) -> None: while self._on_close: diff --git a/src/rootfilespec/rntuple/schema.py b/src/rootfilespec/rntuple/schema.py index 2864986..08c14f0 100644 --- a/src/rootfilespec/rntuple/schema.py +++ b/src/rootfilespec/rntuple/schema.py @@ -3,9 +3,9 @@ from rootfilespec.rntuple.RFrame import RecordFrame from rootfilespec.serializable import serializable -from rootfilespec.structutil import CountedString, Fmt, OptionalField +from rootfilespec.structutil import Fmt, OptionalField, ROOTString -RNTupleString = Annotated[bytes, CountedString(" ReadObjMethod: return ReadObjMethod(membermethod) +def read_value(ftype: Any, buffer: ReadBuffer) -> tuple[MemberType, ReadBuffer]: + """Read one value of the given type from the buffer + + The type is a ROOTSerializable class, or an annotated builtin that a type + lookup can also return: ``TString`` is ``Annotated[bytes, ROOTString("TString")]``, + so a ``TString`` stored as an object of its own reads as ``bytes``. + """ + if isinstance(ftype, type) and issubclass(ftype, ROOTSerializable): + return ftype.read(buffer) + return _build_read(ftype)(buffer) + + def _build_update_members(fname: str, ftype: Any) -> ReadMembersMethod: if isinstance(ftype, type) and issubclass(ftype, ROOTSerializable): return _ReadWrapper(fname, ftype) diff --git a/src/rootfilespec/structutil.py b/src/rootfilespec/structutil.py index 89c813c..80aa160 100644 --- a/src/rootfilespec/structutil.py +++ b/src/rootfilespec/structutil.py @@ -1,6 +1,6 @@ import dataclasses import operator -from typing import get_args +from typing import Literal, get_args from rootfilespec.serializable import Members, MemberSerDe, ReadBuffer, ROOTSerializable @@ -32,32 +32,82 @@ def build_reader(self, fname: str, ftype: type): return _FmtReader(fname, self.fmt, ftype) +StringEncoding = Literal["RNTuple", "TString", "std::string", "charstar"] +"""The on-disk string encodings (root-io-spec Conventions §5) + +- ``"RNTuple"``: a u32 little-endian length, then that many bytes. +- ``"TString"``: the counted string, one length byte, or 255 then a u32 + big-endian length (§5.1). A ``TString`` data member and a ``std::string`` + element of a collection use it. +- ``"std::string"``: a ``std::string`` data member, the counted string inside a + byte count and a version word (§5.3). +- ``"charstar"``: a ``char*`` member (``kCharStar``), an i32 big-endian length, + then that many bytes, with no terminator and no 255 escape (§5.4). A length of + zero or less means no bytes follow, so a null pointer reads as ``b""``. +""" + +_kByteCountMask = 0x40000000 + + +def read_string( + buffer: ReadBuffer, encoding: StringEncoding +) -> tuple[bytes, ReadBuffer]: + """Read one string in the given encoding, as plain ``bytes`` + + The bytes are not decoded: ROOT strings are uninterpreted bytes, and a reader + should not assume an encoding (root-io-spec Conventions §5.1). + """ + if encoding == "RNTuple": + (length,), buffer = buffer.unpack("B") + if length == 255: + (length,), buffer = buffer.unpack(">I") + elif encoding == "std::string": + (bytecount, _version), buffer = buffer.unpack(">IH") + if not bytecount & _kByteCountMask: + msg = f"std::string member without a byte count: {bytecount:#010x}" + raise ValueError(msg) + start = buffer.relpos + data, buffer = read_string(buffer, "TString") + if buffer.relpos - start != (bytecount & ~_kByteCountMask) - 2: + msg = f"std::string byte count {bytecount & ~_kByteCountMask} does not match its {len(data)}-byte string" + raise ValueError(msg) + return data, buffer + else: # "charstar", checked by ROOTString + (length,), buffer = buffer.unpack(">i") + length = max(length, 0) + return buffer.consume(length) + + @dataclasses.dataclass -class _CountedStringReader: +class _ROOTStringReader: fname: str - length_fmt: str + encoding: StringEncoding def __call__( self, members: Members, buffer: ReadBuffer ) -> tuple[Members, ReadBuffer]: - (length,), buffer = buffer.unpack(self.length_fmt) - members[self.fname], buffer = buffer.consume(length) + members[self.fname], buffer = read_string(buffer, self.encoding) return members, buffer @dataclasses.dataclass -class CountedString(MemberSerDe): - """A string stored as its length, then that many bytes, read as plain ``bytes`` +class ROOTString(MemberSerDe): + """A string member, read as plain ``bytes``: ``Annotated[bytes, ROOTString(...)]`` - ``length_fmt`` is the struct format of the length, e.g. ``" None: + if self.encoding not in get_args(StringEncoding): + msg = f"Unknown string encoding {self.encoding!r}" + raise ValueError(msg) def build_reader(self, fname: str, ftype: type): # noqa: ARG002 - return _CountedStringReader(fname, self.length_fmt) + return _ROOTStringReader(fname, self.encoding) @dataclasses.dataclass diff --git a/tests/test_counted_string.py b/tests/test_counted_string.py deleted file mode 100644 index b390cbb..0000000 --- a/tests/test_counted_string.py +++ /dev/null @@ -1,68 +0,0 @@ -from pathlib import Path -from typing import Annotated - -import pytest - -from rootfilespec.bootstrap import BOOTSTRAP_CONTEXT -from rootfilespec.reader import open_path -from rootfilespec.rntuple.RNTuple import RNTuple -from rootfilespec.serializable import ( - BufferContext, - ReadBuffer, - ROOTSerializable, - serializable, -) -from rootfilespec.structutil import CountedString - -DATA = Path(__file__).parent.parent / "reference" / "root-io-spec" / "data" / "rntuple" - - -@serializable -class _Named(ROOTSerializable): - name: Annotated[bytes, CountedString(" ReadBuffer: - return ReadBuffer(memoryview(data), 0, BOOTSTRAP_CONTEXT, BufferContext(abspos=0)) - - -def test_counted_string_is_plain_bytes(): - """Issue #68: a counted string is read as the bytes on disk, not decoded""" - name = "héllo·wörld".encode() - data = len(name).to_bytes(4, "little") + name + b"\x00\x00\x00\x00" + b"rest" - obj, rest = _Named.read(_buffer(data)) - assert obj.name == name - assert type(obj.name) is bytes - assert obj.after == b"" - assert bytes(rest.data) == b"rest" - - -def test_counted_string_too_short(): - with pytest.raises(Exception): # noqa: B017, PT011 - _Named.read(_buffer(b"\x09\x00\x00\x00abc")) - - -@pytest.mark.skipif(not DATA.exists(), reason="reference/root-io-spec not checked out") -def test_rntuple_strings_are_bytes(): - """Every RNTuple string member of every root-io-spec RNTuple fixture is bytes""" - for path in sorted(DATA.glob("*.root")): - with open_path(path) as reader: - keylist = reader.keylist() - for name in keylist: - key = keylist[name] - if key.fClassName.fString != b"ROOT::RNTuple": - continue - rntuple = RNTuple.from_anchor(reader.fetch(key), reader.fetch.buffer) - header = rntuple.headerEnvelope - assert type(header.fName) is bytes - assert header.fName == name - assert type(header.fLibrary) is bytes - for field in rntuple.schemaDescription.fieldDescriptions: - for value in ( - field.fFieldName, - field.fTypeName, - field.fTypeAlias, - field.fFieldDescription, - ): - assert type(value) is bytes diff --git a/tests/test_key_compression_and_names.py b/tests/test_key_compression_and_names.py index 6d7061b..28bf344 100644 --- a/tests/test_key_compression_and_names.py +++ b/tests/test_key_compression_and_names.py @@ -3,7 +3,6 @@ import pytest from rootfilespec.bootstrap import BOOTSTRAP_CONTEXT -from rootfilespec.bootstrap.strings import TString from rootfilespec.bootstrap.TDirectory import TKeyList from rootfilespec.bootstrap.TKey import TKey, TKey_header from rootfilespec.serializable import BufferContext, ReadBuffer @@ -45,8 +44,9 @@ def test_raw_payload_longer_than_objlen(): """ key, record = _key(b"TString", b"s", b"\x03abc\x00\x00", objlen=4) assert not key.header.is_compressed() - obj = key.read_object(lambda _seek, _size: _buffer(record, 100), TString) - assert obj == TString(b"abc") + # A TString stored as an object of its own reads as bytes (#68) + obj = key.read_object(lambda _seek, _size: _buffer(record, 100)) + assert obj == b"abc" def test_compressed_is_objlen_larger_than_payload(): diff --git a/tests/test_read.py b/tests/test_read.py index 8966cfc..77ca701 100644 --- a/tests/test_read.py +++ b/tests/test_read.py @@ -107,33 +107,33 @@ def _walk_branchlist( msg = f"Expected TBranch but got {type(branch).__name__}" raise TypeError(msg) branch = cast(TBranch, branch) - print(f"{' ' * indent}Branch: {path + branch.fName.fString!r}") + print(f"{' ' * indent}Branch: {path + branch.fName!r}") _walk_branchlist( branch.fBranches, fetch, notimplemented_callback, - path=path + branch.fName.fString + b".", + path=path + branch.fName + b".", indent=indent + 1, ) if not hasattr(branch, "fClassName"): continue # Simple data type, we trust we can deserialize branch = cast(TBranchObject, branch) - cpptype = branch.fClassName.fString + cpptype = branch.fClassName if hasattr(branch, "fParentName"): branch = cast(TBranchElement, branch) - if branch.fParentName.fString: + if branch.fParentName: # The split branch is for a base class # apparently the fTitle is the parent branch name + '.' + the type path? # e.g. uproot-issue-798.root (xAOD3a3aFileMetaDataAuxInfo_v1) - cpptype = branch.fTitle.fString.rsplit(b".", 1)[-1] + cpptype = branch.fTitle.rsplit(b".", 1)[-1] typename = normalize(cpptype) print(f"{' ' * indent} Type: {typename}") if len(branch.fLeaves.objects): # This is a split branch leaves = ( - b"Some" if isinstance(leaf, Ref) else leaf.fName.fString + b"Some" if isinstance(leaf, Ref) else leaf.fName for leaf in branch.fLeaves.objects ) print(f"{' ' * indent} Leaves: {b','.join(leaves)!r}") @@ -182,7 +182,7 @@ def _walk( return for item in keylist.values(): - itempath = path + item.fName.fString + itempath = path + item.fName try: obj = reader.fetch(item) except NotImplementedError as ex: diff --git a/tests/test_root_string.py b/tests/test_root_string.py new file mode 100644 index 0000000..89e957f --- /dev/null +++ b/tests/test_root_string.py @@ -0,0 +1,220 @@ +from pathlib import Path +from typing import Annotated + +import pytest + +from rootfilespec.bootstrap import BOOTSTRAP_CONTEXT, TString +from rootfilespec.bootstrap.TFile import InitialReadLocator +from rootfilespec.bootstrap.TStreamerInfo import TStreamerInfo +from rootfilespec.container import StdVector +from rootfilespec.dynamic import streamerinfo_to_classes +from rootfilespec.reader import Fetcher, open_path +from rootfilespec.rntuple.RNTuple import RNTuple +from rootfilespec.serializable import ( + BufferContext, + ReadBuffer, + ROOTSerializable, + serializable, +) +from rootfilespec.structutil import ROOTString, StringEncoding, read_string + +DATA = Path(__file__).parent.parent / "reference" / "root-io-spec" / "data" + +pytestmark = pytest.mark.skipif( + not DATA.exists(), reason="reference/root-io-spec not checked out" +) + + +def _buffer(data: bytes, abspos: int = 0) -> ReadBuffer: + return ReadBuffer( + memoryview(data), 0, BOOTSTRAP_CONTEXT, BufferContext(abspos=abspos) + ) + + +def _read(encoding: StringEncoding, data: bytes) -> tuple[bytes, bytes]: + value, rest = read_string(_buffer(data), encoding) + return value, bytes(rest.data) + + +@serializable +class _Named(ROOTSerializable): + name: Annotated[bytes, ROOTString("RNTuple")] + after: Annotated[bytes, ROOTString("RNTuple")] + + +def test_rntuple_string_is_plain_bytes(): + """Issue #68: a string is read as the bytes on disk, not decoded""" + name = "héllo·wörld".encode() + data = len(name).to_bytes(4, "little") + name + b"\x00\x00\x00\x00" + b"rest" + obj, rest = _Named.read(_buffer(data)) + assert obj.name == name + assert type(obj.name) is bytes + assert obj.after == b"" + assert bytes(rest.data) == b"rest" + + +def test_string_too_short(): + with pytest.raises(Exception): # noqa: B017, PT011 + _Named.read(_buffer(b"\x09\x00\x00\x00abc")) + + +@pytest.mark.parametrize( + ("data", "value"), + [ + (b"\x00", b""), + (b"\x05hello", b"hello"), + (b"\xfe" + b"x" * 254, b"x" * 254), + # a 255-character string is always in the long form (Conventions §5.1) + (b"\xff\x00\x00\x00\xff" + b"y" * 255, b"y" * 255), + (b"\xff\x00\x00\x01\x2c" + b"z" * 300, b"z" * 300), + ], +) +def test_tstring_encoding(data: bytes, value: bytes): + """Conventions §5.1: one length byte, or 255 then a 4-byte length""" + assert _read("TString", data + b"rest") == (value, b"rest") + + +@pytest.mark.parametrize( + ("data", "value"), + [ + (b"\x00\x00\x00\x02hi", b"hi"), + # no 255 escape: a length byte of 0xff is part of the i32 + (b"\x00\x00\x00\xff" + b"w" * 255, b"w" * 255), + # a null pointer and an empty string are both four zero bytes + (b"\x00\x00\x00\x00", b""), + # n <= 0 means nothing follows + (b"\xff\xff\xff\xff", b""), + ], +) +def test_charstar_encoding(data: bytes, value: bytes): + """Conventions §5.4 and #20: an i32 length, then that many bytes""" + assert _read("charstar", data + b"rest") == (value, b"rest") + + +def test_std_string_encoding(): + """Conventions §5.3: a counted string inside a byte count and version word""" + assert _read("std::string", b"\x40\x00\x00\x06\x00\x0a\x03abcrest") == ( + b"abc", + b"rest", + ) + # an empty std::string member is 7 bytes, not 1 + assert _read("std::string", b"\x40\x00\x00\x03\x00\x0a\x00") == (b"", b"") + with pytest.raises(ValueError, match="without a byte count"): + _read("std::string", b"\x00\x00\x00\x06\x00\x0a\x03abc") + with pytest.raises(ValueError, match="does not match"): + _read("std::string", b"\x40\x00\x00\x07\x00\x0a\x03abcd") + + +def test_rntuple_strings_are_bytes(): + """Every RNTuple string member of every root-io-spec RNTuple fixture is bytes""" + for path in sorted((DATA / "rntuple").glob("*.root")): + with open_path(path) as reader: + keylist = reader.keylist() + for name in keylist: + key = keylist[name] + if key.fClassName != b"ROOT::RNTuple": + continue + rntuple = RNTuple.from_anchor(reader.fetch(key), reader.fetch.buffer) + header = rntuple.headerEnvelope + assert type(header.fName) is bytes + assert header.fName == name + assert type(header.fLibrary) is bytes + for field in rntuple.schemaDescription.fieldDescriptions: + for value in ( + field.fFieldName, + field.fTypeName, + field.fTypeAlias, + field.fFieldDescription, + ): + assert type(value) is bytes + + +def test_key_and_named_strings_are_bytes(): + """TKey names and TNamed members are bytes: `container/file-minimal` holds a + TObjString named `s` whose string is `hello` (Conventions §5.1)""" + with open_path(DATA / "container" / "file-minimal.root") as reader: + keylist = reader.keylist() + assert all(type(name) is bytes for name in keylist) + for key in keylist.values(): + assert type(key.fClassName) is bytes + assert type(key.fTitle) is bytes + infos = reader.streamerinfos() + assert all(type(name) is bytes for name in infos) + assert all(type(info.fTitle) is bytes for info in infos.values()) + + +def _streamerinfo(path: Path, classname: bytes) -> TStreamerInfo: + """A class's TStreamerInfo, read with the bootstrap types only, for a file + whose other classes cannot all be generated yet""" + raw = path.read_bytes() + fetch = Fetcher(lambda offset, size: raw[offset : offset + size], BOOTSTRAP_CONTEXT) + file = fetch(InitialReadLocator()) + assert file.streamerinfo_locator is not None + streamerinfo = fetch.resolve(file.streamerinfo_locator) + (info,) = ( + item + for item in streamerinfo.items + if isinstance(item, TStreamerInfo) and item.fName == classname + ) + return info + + +def test_generated_string_members(): + """Generated classes annotate string members with ROOTString: a std::string + member is framed, a vector's elements are bare (root-io-spec + serialization/collections), and a char* member is kCharStar + (serialization/element-types)""" + path = DATA / "serialization" / "collections.root" + with open_path(path) as reader: + assert reader.streamerinfo is not None + code = streamerinfo_to_classes(reader.streamerinfo) + assert "fStr: Annotated[bytes, ROOTString('std::string')]" in code + assert "fWords: StdVector[Annotated[bytes, ROOTString('TString')]]" in code + + info = _streamerinfo(DATA / "serialization" / "element-types.root", b"ElementZoo") + for name in (b"fText", b"fNull"): + mdef, deps = info.element(name).member_definition(parent=info) + assert mdef == f"{name.decode()}: Annotated[bytes, ROOTString('charstar')]" + assert deps == [] + + +@serializable +class _CollectionStrings(ROOTSerializable): + fWords: StdVector[TString] + + +@serializable +class _StdStringMember(ROOTSerializable): + fStr: Annotated[bytes, ROOTString("std::string")] + + +def test_fixture_string_bytes(): + """The string members of root-io-spec's uncompressed fixtures, read at the + offsets their case.toml pins""" + raw = (DATA / "serialization" / "collections.root").read_bytes() + words, _ = _CollectionStrings.read(_buffer(raw[453:], 453)) + assert words.fWords.items == [b"pq", b"rs"] + member, _ = _StdStringMember.read(_buffer(raw[501:], 501)) + assert member.fStr == b"abc" + + raw = (DATA / "serialization" / "element-types.root").read_bytes() + text, rest = read_string(_buffer(raw[352:], 352), "charstar") + assert text == b"hi" + null, _ = read_string(rest, "charstar") + assert null == b"" + + +def test_string_records(): + """A TString, TStringLong or std::string stored as an object of its own reads + as bytes: root-io-spec serialization/unframed-records and + serialization/stringlong (Conventions §5.1, §5.1.1)""" + with open_path(DATA / "serialization" / "unframed-records.root") as reader: + keylist = reader.keylist() + assert reader.fetch(keylist[b"tstring"]) == b"hello" + assert reader.fetch(keylist[b"tstringlong"]) == b"a long string" + with open_path(DATA / "serialization" / "stringlong.root") as reader: + keylist = reader.keylist() + small = reader.fetch(keylist[b"small"]) + assert (small.fLong, small.fPlain) == (b"abcdef", b"abcdef") + big = reader.fetch(keylist[b"big"]) + assert (big.fLong, big.fPlain) == (b"x" * 300, b"x" * 300) diff --git a/tests/test_spec_cases.py b/tests/test_spec_cases.py index d0f5cde..973f203 100644 --- a/tests/test_spec_cases.py +++ b/tests/test_spec_cases.py @@ -282,8 +282,8 @@ def test_case_records(case: str, trace: Trace): for offset, key in found.items(): record = records[offset] assert key.header.fNbytes == record["nbytes"] - assert key.fClassName.fString.decode() == record["class"] + assert key.fClassName.decode() == record["class"] if "name" in record: - assert key.fName.fString.decode() == record["name"] + assert key.fName.decode() == record["name"] assert key.header.fCycle == record["cycle"] assert key.fSeekPdir == record["seek_pdir"] diff --git a/tests/test_spec_fixtures.py b/tests/test_spec_fixtures.py index f61f57c..3f398ff 100644 --- a/tests/test_spec_fixtures.py +++ b/tests/test_spec_fixtures.py @@ -89,16 +89,13 @@ class ExpectedFailure(NamedTuple): "serialization/references.root": ExpectedFailure( (101,), ValueError, "Unknown type TRef " ), - # The TStringLong member of SText is left uninterpreted and its bytes are - # then read as the next member - "serialization/stringlong.root": ExpectedFailure( - (101,), IndexError, "Cannot get slice" + # The TDatime, TString and TStringLong records read; a TObject record has + # no byte count, which the reader requires + "serialization/unframed-records.root": ExpectedFailure( + (123,), ValueError, "fByteCount is 0" ), # TH1L derives from the missing TArrayL64 (#107); its version word of 0 is # also misread as announcing a checksum (#111) - "serialization/unframed-records.root": ExpectedFailure( - (123, 101), AttributeError, "type object 'int' has no attribute 'read'" - ), "serialization/version-zero.root": ExpectedFailure( (107, 111), TypeError, r"__init__\(\) missing 36 required positional arguments" ), @@ -153,7 +150,7 @@ def test_tlist_options(): with open_path(DATA / "serialization/object-tags.root") as reader: lst = reader.fetch(reader.keylist()[b"lst"]) assert isinstance(lst, TList) - assert lst.fName.fString == b"lst" + assert lst.fName == b"lst" assert len(lst.items) == 4 assert lst.options == [b"", b"", b"opt", b""] diff --git a/tests/test_tdatime.py b/tests/test_tdatime.py index f3b3310..a90b1bc 100644 --- a/tests/test_tdatime.py +++ b/tests/test_tdatime.py @@ -1,6 +1,8 @@ from datetime import datetime +from pathlib import Path from rootfilespec.bootstrap.TDatime import TDatime_to_datetime +from rootfilespec.reader import open_path def _pack(year: int, month: int, day: int, hour: int, minute: int, second: int): @@ -26,3 +28,20 @@ def test_invalid_datime_is_none(): assert TDatime_to_datetime(0) is None assert TDatime_to_datetime(_pack(2026, 13, 1, 0, 0, 0)) is None assert TDatime_to_datetime(_pack(2026, 2, 30, 0, 0, 0)) is None + + +def test_tdatime_record(): + """A TDatime stored as a record is the four bytes of fDatime and nothing else + (root-io-spec serialization/unframed-records, 2026-09-22 12:00:00)""" + path = ( + Path(__file__).parent.parent + / "reference" + / "root-io-spec" + / "data" + / "serialization" + / "unframed-records.root" + ) + with open_path(path) as reader: + value = reader.fetch(reader.keylist()[b"datime"]) + assert value == 0x7E6CC000 + assert TDatime_to_datetime(value) == datetime(2026, 9, 22, 12, 0, 0) From 753e224f23019b00578fc701085ac8f4840cb9b7 Mon Sep 17 00:00:00 2001 From: Samantha Abbott <52800387+sathabbott@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:19:16 -0500 Subject: [PATCH 5/9] Make a std::string member's frame a ROOTString option, not an encoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A std::string data member is the TString encoding inside the usual byte count and version word (root-io-spec Conventions §5.3). The frame is not a fourth string encoding, so: - StringEncoding is back to the three length formats Nick listed on #127: "RNTuple", "TString", "charstar". - ROOTString(encoding, framed=False). With framed=True it reads the frame with StreamHeader, as StdVector and the other framed members do, and checks the byte count, instead of parsing the frame itself with a second copy of kByteCountMask. - read_string() is gone. ROOTString.read(buffer) is the one string reader: the member reader calls it, and TKey and TList call it for their hand-read strings. - read_value() takes type[MemberType] | type[ROOTSerializable], as Nick suggested, and goes through _build_read for both kinds of type. - docs/design.md states the design assumption Nick raised: the bytes don't record their encoding; the annotation, key class name or class tag that holds them does, and writing a string back needs it. - test_tdatime_record is dropped: the TDatime record reads as a side effect of read_value, but it belongs to #123, not #68. Assisted-by: claude-code:claude-opus-5-5 --- docs/design.md | 10 ++- src/rootfilespec/bootstrap/TKey.py | 8 +- src/rootfilespec/bootstrap/TList.py | 4 +- src/rootfilespec/bootstrap/TStreamerInfo.py | 5 +- src/rootfilespec/bootstrap/strings.py | 5 +- src/rootfilespec/serializable.py | 6 +- src/rootfilespec/structutil.py | 94 +++++++++++---------- tests/test_root_string.py | 31 ++++--- tests/test_tdatime.py | 19 ----- 9 files changed, 89 insertions(+), 93 deletions(-) diff --git a/docs/design.md b/docs/design.md index 519f21b..dc0f943 100644 --- a/docs/design.md +++ b/docs/design.md @@ -111,10 +111,16 @@ class TNamed(TObject): 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 on-disk form (`"TString"`, `"std::string"`, `"charstar"` and -`"RNTuple"`), and `bootstrap.TString` is the alias +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 encoding they came +from. That stays with whatever holds them: the member's annotation, a key's +class name, or a streamed object's class tag. Writing a string back out +therefore needs that context, not just the `bytes`. + ## Data Fetching and Locators Many ROOT objects serve as references to data stored elsewhere in the file. For diff --git a/src/rootfilespec/bootstrap/TKey.py b/src/rootfilespec/bootstrap/TKey.py index 5e228eb..458b281 100644 --- a/src/rootfilespec/bootstrap/TKey.py +++ b/src/rootfilespec/bootstrap/TKey.py @@ -14,7 +14,7 @@ read_value, serializable, ) -from rootfilespec.structutil import Fmt, read_string +from rootfilespec.structutil import Fmt, ROOTString @serializable @@ -92,9 +92,9 @@ def update_members( (fSeekKey, fSeekPdir), buffer = buffer.unpack(">ii") else: (fSeekKey, fSeekPdir), buffer = buffer.unpack(">qq") - fClassName, buffer = read_string(buffer, "TString") - fName, buffer = read_string(buffer, "TString") - fTitle, buffer = read_string(buffer, "TString") + 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) diff --git a/src/rootfilespec/bootstrap/TList.py b/src/rootfilespec/bootstrap/TList.py index 7da73e7..6c615a1 100644 --- a/src/rootfilespec/bootstrap/TList.py +++ b/src/rootfilespec/bootstrap/TList.py @@ -9,7 +9,7 @@ ROOTSerializable, serializable, ) -from rootfilespec.structutil import Fmt, read_string +from rootfilespec.structutil import Fmt, ROOTString @serializable @@ -47,7 +47,7 @@ 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 = read_string(buffer, "TString") + option, buffer = ROOTString("TString").read(buffer) items.append(item) options.append(option) members["items"] = items diff --git a/src/rootfilespec/bootstrap/TStreamerInfo.py b/src/rootfilespec/bootstrap/TStreamerInfo.py index bf5a7b2..fccdb00 100644 --- a/src/rootfilespec/bootstrap/TStreamerInfo.py +++ b/src/rootfilespec/bootstrap/TStreamerInfo.py @@ -608,4 +608,7 @@ class TStreamerSTLstring(TStreamerSTL): def member_definition(self, parent: TStreamerInfo): # noqa: ARG002 # A std::string data member has a byte count and version word (Conventions §5.3) - return f"{self.member_name()}: Annotated[bytes, ROOTString('std::string')]", [] + return ( + f"{self.member_name()}: Annotated[bytes, ROOTString('TString', framed=True)]", + [], + ) diff --git a/src/rootfilespec/bootstrap/strings.py b/src/rootfilespec/bootstrap/strings.py index 5542462..8f1a138 100644 --- a/src/rootfilespec/bootstrap/strings.py +++ b/src/rootfilespec/bootstrap/strings.py @@ -8,7 +8,7 @@ A counted string: one length byte, or 255 then a 4-byte length, then the bytes (root-io-spec Conventions §5.1). ``TString`` has no Python class: a member is ``bytes``, and the encoding lives in the annotation. Read a bare one by hand -with ``read_string(buffer, "TString")``. +with ``ROOTString("TString").read(buffer)``. """ TStringLong = Annotated[bytes, ROOTString("charstar")] @@ -23,5 +23,6 @@ """A ``std::string`` looked up by name, as an object of its own (a key's class) It is the bare counted string, as a ``TString`` is. A ``std::string`` data member -has a byte count and version word first: ``Annotated[bytes, ROOTString("std::string")]``. +has a byte count and version word first: +``Annotated[bytes, ROOTString("TString", framed=True)]``. """ diff --git a/src/rootfilespec/serializable.py b/src/rootfilespec/serializable.py index 649a793..9bf9b68 100644 --- a/src/rootfilespec/serializable.py +++ b/src/rootfilespec/serializable.py @@ -280,15 +280,15 @@ def _build_read(ftype: type[MemberType]) -> ReadObjMethod: return ReadObjMethod(membermethod) -def read_value(ftype: Any, buffer: ReadBuffer) -> tuple[MemberType, ReadBuffer]: +def read_value( + ftype: type[MemberType] | type["ROOTSerializable"], buffer: ReadBuffer +) -> tuple[MemberType, ReadBuffer]: """Read one value of the given type from the buffer The type is a ROOTSerializable class, or an annotated builtin that a type lookup can also return: ``TString`` is ``Annotated[bytes, ROOTString("TString")]``, so a ``TString`` stored as an object of its own reads as ``bytes``. """ - if isinstance(ftype, type) and issubclass(ftype, ROOTSerializable): - return ftype.read(buffer) return _build_read(ftype)(buffer) diff --git a/src/rootfilespec/structutil.py b/src/rootfilespec/structutil.py index 80aa160..47ec16e 100644 --- a/src/rootfilespec/structutil.py +++ b/src/rootfilespec/structutil.py @@ -32,82 +32,84 @@ def build_reader(self, fname: str, ftype: type): return _FmtReader(fname, self.fmt, ftype) -StringEncoding = Literal["RNTuple", "TString", "std::string", "charstar"] +StringEncoding = Literal["RNTuple", "TString", "charstar"] """The on-disk string encodings (root-io-spec Conventions §5) - ``"RNTuple"``: a u32 little-endian length, then that many bytes. - ``"TString"``: the counted string, one length byte, or 255 then a u32 - big-endian length (§5.1). A ``TString`` data member and a ``std::string`` - element of a collection use it. -- ``"std::string"``: a ``std::string`` data member, the counted string inside a - byte count and a version word (§5.3). -- ``"charstar"``: a ``char*`` member (``kCharStar``), an i32 big-endian length, - then that many bytes, with no terminator and no 255 escape (§5.4). A length of - zero or less means no bytes follow, so a null pointer reads as ``b""``. + big-endian length (§5.1). ``TString`` and ``std::string`` use it. +- ``"charstar"``: an i32 big-endian length, then that many bytes, with no + terminator and no 255 escape: a ``char*`` member (``kCharStar``, §5.4) and a + ``TStringLong`` (§5.1.1). A length of zero or less means no bytes follow, so a + null pointer reads as ``b""``. """ -_kByteCountMask = 0x40000000 - - -def read_string( - buffer: ReadBuffer, encoding: StringEncoding -) -> tuple[bytes, ReadBuffer]: - """Read one string in the given encoding, as plain ``bytes`` - - The bytes are not decoded: ROOT strings are uninterpreted bytes, and a reader - should not assume an encoding (root-io-spec Conventions §5.1). - """ - if encoding == "RNTuple": - (length,), buffer = buffer.unpack("B") - if length == 255: - (length,), buffer = buffer.unpack(">I") - elif encoding == "std::string": - (bytecount, _version), buffer = buffer.unpack(">IH") - if not bytecount & _kByteCountMask: - msg = f"std::string member without a byte count: {bytecount:#010x}" - raise ValueError(msg) - start = buffer.relpos - data, buffer = read_string(buffer, "TString") - if buffer.relpos - start != (bytecount & ~_kByteCountMask) - 2: - msg = f"std::string byte count {bytecount & ~_kByteCountMask} does not match its {len(data)}-byte string" - raise ValueError(msg) - return data, buffer - else: # "charstar", checked by ROOTString - (length,), buffer = buffer.unpack(">i") - length = max(length, 0) - return buffer.consume(length) - @dataclasses.dataclass class _ROOTStringReader: fname: str - encoding: StringEncoding + serde: "ROOTString" def __call__( self, members: Members, buffer: ReadBuffer ) -> tuple[Members, ReadBuffer]: - members[self.fname], buffer = read_string(buffer, self.encoding) + members[self.fname], buffer = self.serde.read(buffer) return members, buffer @dataclasses.dataclass class ROOTString(MemberSerDe): - """A string member, read as plain ``bytes``: ``Annotated[bytes, ROOTString(...)]`` + """A string, read as plain ``bytes``: ``Annotated[bytes, ROOTString(...)]`` - ``encoding`` is one of the on-disk encodings of :data:`StringEncoding`. + The bytes are not decoded: ROOT strings are uninterpreted bytes, and a reader + should not assume an encoding (root-io-spec Conventions §5.1). Nor do they + say which encoding they came from: that stays with whatever holds them (the + annotation, a key's class name, a streamed object's class tag). """ encoding: StringEncoding + """How the length and the bytes are stored, one of :data:`StringEncoding`""" + framed: bool = False + """Whether a byte count and version word come first, as for a ``std::string`` + data member (§5.3). Read with :class:`StreamHeader`, as other framed members are.""" def __post_init__(self) -> None: if self.encoding not in get_args(StringEncoding): msg = f"Unknown string encoding {self.encoding!r}" raise ValueError(msg) + def read(self, buffer: ReadBuffer) -> tuple[bytes, ReadBuffer]: + """Read one string from the buffer""" + if self.framed: + return self._read_framed(buffer) + if self.encoding == "RNTuple": + (length,), buffer = buffer.unpack("B") + if length == 255: + (length,), buffer = buffer.unpack(">I") + else: # "charstar", checked in __post_init__ + (length,), buffer = buffer.unpack(">i") + length = max(length, 0) + return buffer.consume(length) + + def _read_framed(self, buffer: ReadBuffer) -> tuple[bytes, ReadBuffer]: + # Imported here: rootfilespec.bootstrap imports this module (#119) + from rootfilespec.bootstrap.streamedobject import StreamHeader + + header, buffer = StreamHeader.read(buffer) + if header.fVersion is None: + msg = f"Expected a byte count and version before the string, got {header}" + raise ValueError(msg) + start = buffer.relpos + data, buffer = ROOTString(self.encoding).read(buffer) + if buffer.relpos - start != header.fByteCount - 2: + msg = f"String byte count {header.fByteCount} does not match its {len(data)}-byte string" + raise ValueError(msg) + return data, buffer + def build_reader(self, fname: str, ftype: type): # noqa: ARG002 - return _ROOTStringReader(fname, self.encoding) + return _ROOTStringReader(fname, self) @dataclasses.dataclass diff --git a/tests/test_root_string.py b/tests/test_root_string.py index 89e957f..9a9e0b0 100644 --- a/tests/test_root_string.py +++ b/tests/test_root_string.py @@ -16,7 +16,7 @@ ROOTSerializable, serializable, ) -from rootfilespec.structutil import ROOTString, StringEncoding, read_string +from rootfilespec.structutil import ROOTString, StringEncoding DATA = Path(__file__).parent.parent / "reference" / "root-io-spec" / "data" @@ -31,8 +31,10 @@ def _buffer(data: bytes, abspos: int = 0) -> ReadBuffer: ) -def _read(encoding: StringEncoding, data: bytes) -> tuple[bytes, bytes]: - value, rest = read_string(_buffer(data), encoding) +def _read( + encoding: StringEncoding, data: bytes, framed: bool = False +) -> tuple[bytes, bytes]: + value, rest = ROOTString(encoding, framed).read(_buffer(data)) return value, bytes(rest.data) @@ -91,18 +93,19 @@ def test_charstar_encoding(data: bytes, value: bytes): assert _read("charstar", data + b"rest") == (value, b"rest") -def test_std_string_encoding(): - """Conventions §5.3: a counted string inside a byte count and version word""" - assert _read("std::string", b"\x40\x00\x00\x06\x00\x0a\x03abcrest") == ( +def test_framed_string(): + """Conventions §5.3: a std::string data member is a counted string inside a + byte count and version word""" + assert _read("TString", b"\x40\x00\x00\x06\x00\x0a\x03abcrest", framed=True) == ( b"abc", b"rest", ) # an empty std::string member is 7 bytes, not 1 - assert _read("std::string", b"\x40\x00\x00\x03\x00\x0a\x00") == (b"", b"") - with pytest.raises(ValueError, match="without a byte count"): - _read("std::string", b"\x00\x00\x00\x06\x00\x0a\x03abc") + assert _read("TString", b"\x40\x00\x00\x03\x00\x0a\x00", framed=True) == (b"", b"") + with pytest.raises(ValueError, match="Expected a byte count and version"): + _read("TString", b"\x00\x00\x00\x06\x00\x0a\x03abc", framed=True) with pytest.raises(ValueError, match="does not match"): - _read("std::string", b"\x40\x00\x00\x07\x00\x0a\x03abcd") + _read("TString", b"\x40\x00\x00\x07\x00\x0a\x03abcd", framed=True) def test_rntuple_strings_are_bytes(): @@ -168,7 +171,7 @@ def test_generated_string_members(): with open_path(path) as reader: assert reader.streamerinfo is not None code = streamerinfo_to_classes(reader.streamerinfo) - assert "fStr: Annotated[bytes, ROOTString('std::string')]" in code + assert "fStr: Annotated[bytes, ROOTString('TString', framed=True)]" in code assert "fWords: StdVector[Annotated[bytes, ROOTString('TString')]]" in code info = _streamerinfo(DATA / "serialization" / "element-types.root", b"ElementZoo") @@ -185,7 +188,7 @@ class _CollectionStrings(ROOTSerializable): @serializable class _StdStringMember(ROOTSerializable): - fStr: Annotated[bytes, ROOTString("std::string")] + fStr: Annotated[bytes, ROOTString("TString", framed=True)] def test_fixture_string_bytes(): @@ -198,9 +201,9 @@ def test_fixture_string_bytes(): assert member.fStr == b"abc" raw = (DATA / "serialization" / "element-types.root").read_bytes() - text, rest = read_string(_buffer(raw[352:], 352), "charstar") + text, rest = ROOTString("charstar").read(_buffer(raw[352:], 352)) assert text == b"hi" - null, _ = read_string(rest, "charstar") + null, _ = ROOTString("charstar").read(rest) assert null == b"" diff --git a/tests/test_tdatime.py b/tests/test_tdatime.py index a90b1bc..f3b3310 100644 --- a/tests/test_tdatime.py +++ b/tests/test_tdatime.py @@ -1,8 +1,6 @@ from datetime import datetime -from pathlib import Path from rootfilespec.bootstrap.TDatime import TDatime_to_datetime -from rootfilespec.reader import open_path def _pack(year: int, month: int, day: int, hour: int, minute: int, second: int): @@ -28,20 +26,3 @@ def test_invalid_datime_is_none(): assert TDatime_to_datetime(0) is None assert TDatime_to_datetime(_pack(2026, 13, 1, 0, 0, 0)) is None assert TDatime_to_datetime(_pack(2026, 2, 30, 0, 0, 0)) is None - - -def test_tdatime_record(): - """A TDatime stored as a record is the four bytes of fDatime and nothing else - (root-io-spec serialization/unframed-records, 2026-09-22 12:00:00)""" - path = ( - Path(__file__).parent.parent - / "reference" - / "root-io-spec" - / "data" - / "serialization" - / "unframed-records.root" - ) - with open_path(path) as reader: - value = reader.fetch(reader.keylist()[b"datime"]) - assert value == 0x7E6CC000 - assert TDatime_to_datetime(value) == datetime(2026, 9, 22, 12, 0, 0) From 773c06b85594404ff9bf77d9986f1d467b655003 Mon Sep 17 00:00:00 2001 From: Samantha Abbott <52800387+sathabbott@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:41:16 -0500 Subject: [PATCH 6/9] Read a TypedTKey's object with read_value, like an untyped key's TypedTKey.read stores the looked-up type as objtype, and after #68 a lookup of "TString", "TStringLong" or "string" returns an annotated alias, not a class. read_object called objtype.read() on it and raised AttributeError, so a TypedTKey of a string record could not be read, although fetching the same key untyped returned its bytes. Both branches now read through read_value. Test: the "tstring" record of root-io-spec serialization/unframed-records through TypedTKey reads b"hello". Assisted-by: claude-code:claude-opus-5-5 --- src/rootfilespec/bootstrap/TKey.py | 15 +++++++++------ tests/test_root_string.py | 12 ++++++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/rootfilespec/bootstrap/TKey.py b/src/rootfilespec/bootstrap/TKey.py index 458b281..87dbd72 100644 --- a/src/rootfilespec/bootstrap/TKey.py +++ b/src/rootfilespec/bootstrap/TKey.py @@ -139,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) - else: + readtype: type[ObjType] | type[ROOTSerializable] + if objtype is None: typename = normalize(self.fClassName) - dyntype = buffer.file_context.type_by_name(typename) - obj, buffer = read_value(dyntype, buffer) + readtype = buffer.file_context.type_by_name(typename) + else: + 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 diff --git a/tests/test_root_string.py b/tests/test_root_string.py index 9a9e0b0..4a7ff99 100644 --- a/tests/test_root_string.py +++ b/tests/test_root_string.py @@ -5,6 +5,7 @@ from rootfilespec.bootstrap import BOOTSTRAP_CONTEXT, TString from rootfilespec.bootstrap.TFile import InitialReadLocator +from rootfilespec.bootstrap.TKey import TypedTKey from rootfilespec.bootstrap.TStreamerInfo import TStreamerInfo from rootfilespec.container import StdVector from rootfilespec.dynamic import streamerinfo_to_classes @@ -221,3 +222,14 @@ def test_string_records(): assert (small.fLong, small.fPlain) == (b"abcdef", b"abcdef") big = reader.fetch(keylist[b"big"]) assert (big.fLong, big.fPlain) == (b"x" * 300, b"x" * 300) + + +def test_typed_key_string_record(): + """A TypedTKey reads a string record too: the looked-up type is the + TString alias, not a class, and read_object reads both kinds alike""" + with open_path(DATA / "serialization" / "unframed-records.root") as reader: + key = reader.keylist()[b"tstring"] + buffer = reader.fetch.buffer(key) + typed, _ = TypedTKey.read(buffer) + assert typed.objtype == TString + assert typed.read_from(buffer) == b"hello" From 48c22eb08261618a0e254e4b9252d224c047834f Mon Sep 17 00:00:00 2001 From: Samantha Abbott <52800387+sathabbott@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:48:55 -0500 Subject: [PATCH 7/9] Fix the update_members typo in docs/design.md "read_mupdate_membersembers" was a garbled "update_members", as Nick noted on #127. Assisted-by: claude-code:claude-opus-5-5 --- docs/design.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/design.md b/docs/design.md index dc0f943..7db7b50 100644 --- a/docs/design.md +++ b/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`: From 7be7a721d7b1e8126b51416b060a48225a7b6161 Mon Sep 17 00:00:00 2001 From: Samantha Abbott <52800387+sathabbott@users.noreply.github.com> Date: Wed, 30 Sep 2026 19:01:13 -0500 Subject: [PATCH 8/9] Say that a string read through a pointer loses its ROOT type The design note claimed a streamed object's class tag keeps a string's type, like an annotation or a key's class name does. It doesn't: read_streamed_item reads the tag to look the type up and then returns bare bytes. Members, container elements and keyed records keep the type; a string read through a pointer can't be written back as read. Tracked in #135, as Nick noted on #127 and in #134's AGENTS.md. Assisted-by: claude-code:claude-opus-5-5 --- docs/design.md | 10 ++++++---- src/rootfilespec/structutil.py | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/design.md b/docs/design.md index 7db7b50..f77eb13 100644 --- a/docs/design.md +++ b/docs/design.md @@ -116,10 +116,12 @@ encoding per length format (`"TString"`, `"charstar"` and `"RNTuple"`), and `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 encoding they came -from. That stays with whatever holds them: the member's annotation, a key's -class name, or a streamed object's class tag. Writing a string back out -therefore needs that context, not just the `bytes`. +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 diff --git a/src/rootfilespec/structutil.py b/src/rootfilespec/structutil.py index 47ec16e..8f026c8 100644 --- a/src/rootfilespec/structutil.py +++ b/src/rootfilespec/structutil.py @@ -63,8 +63,8 @@ class ROOTString(MemberSerDe): The bytes are not decoded: ROOT strings are uninterpreted bytes, and a reader should not assume an encoding (root-io-spec Conventions §5.1). Nor do they - say which encoding they came from: that stays with whatever holds them (the - annotation, a key's class name, a streamed object's class tag). + say which encoding they came from: the annotation, or a key's class name, + holds that. A string read through a pointer loses it (``docs/design.md``). """ encoding: StringEncoding From 7846ed29e6f1529256c4e4295d66a2124878c6f7 Mon Sep 17 00:00:00 2001 From: Samantha Abbott <52800387+sathabbott@users.noreply.github.com> Date: Wed, 30 Sep 2026 19:19:47 -0500 Subject: [PATCH 9/9] Compare key class names as bytes in the page checksum tests, after #128 #128's tests index key.fClassName.fString; after #68 the class name is plain bytes. Assisted-by: claude-code:claude-opus-5-5 --- tests/test_page_checksums.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_page_checksums.py b/tests/test_page_checksums.py index bf2bed0..8bf0cc1 100644 --- a/tests/test_page_checksums.py +++ b/tests/test_page_checksums.py @@ -22,7 +22,7 @@ def _page_descriptions(path: Path) -> list[RPageDescription]: keylist = reader.keylist() for name in keylist: key = keylist[name] - if key.fClassName.fString != b"ROOT::RNTuple": + if key.fClassName != b"ROOT::RNTuple": continue rntuple = RNTuple.from_anchor(reader.fetch(key), reader.fetch.buffer) for pagelist in rntuple.pagelistEnvelopes: