[rntuple] Give page descriptions their column, field and global cluster ID - #131
Open
sathabbott wants to merge 2 commits into
Open
sathabbott wants to merge 2 commits into
sathabbott wants to merge 2 commits into
Conversation
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")], exported from
rntuple.schema as RNTupleString, and the eight RNTuple string members (the
header's name, description and library; a field's name, type name, type
alias and description; extra type information's type name) use it.
RString is removed.
This is the RNTuple row of nsmith-#68. The TString, std::string and char* rows
stay open there; char* (nsmith-#20) can use CountedString(">i").
Part of nsmith-#68
Assisted-by: claude-code:claude-opus-5-5
RNTuple.get_extended_page_descriptions(): - returns one entry per physical column in every cluster, so a position is the column ID, as the spec guarantees (Page Locations: the outer items follow column order). With the old default includeSuppressed=False, suppressed columns were dropped and positions shifted. The parameter is removed; a suppressed column is an entry with no pages. The zip over columns is strict, as suggested on nsmith-#49. - gives each page its RNTuple-wide cluster ID. Cluster IDs continue across cluster groups (Page List Envelope); the new RNTuple.firstClusterIDs is the running sum of the footer's fNClusters. Each page list's cluster count is checked against its group. - gives each page its column ID, field ID, FieldDescription and qualified field path. SchemaDescription.field_path(field_id) walks fParentFieldID up to the top-level field (which names itself as parent) and joins the names with "." (forbidden in names, so unambiguous). It returns bytes, like the names, and raises on an out-of-range ID or a cycle. The schema is built once per call; the schemaDescription property itself is unchanged. Fixes nsmith-#49 Fixes nsmith-#116 Assisted-by: claude-code:claude-opus-5-5
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #131 +/- ##
==========================================
+ Coverage 91.82% 91.91% +0.08%
==========================================
Files 37 37
Lines 2962 3019 +57
==========================================
+ Hits 2720 2775 +55
- Misses 242 244 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #49 and #116. Depends on #127 (field names are plain
bytesthere); until #127 merges, this PR also shows its commit.RNTuple.get_extended_page_descriptions()includeSuppressed=False, suppressed columns were dropped and positions shifted. Intest_multiple_representations_rntuple_v1-0-0-0.root, cluster 1's only entry was column 1, at position 0. The parameter is removed: a suppressed column is simply an entry with no pages. The columnzipisstrict=True, as you suggested on [rntuple] Add Field information to InterpretablePage class #49.clusterID. The newRNTuple.firstClusterIDsis the running sum of the footer'sClusterGroup.fNClusters(Page List Envelope), and each page list's cluster count is checked against its group's.InterpretablePagealso carriescolumnID,fieldID(fromfFieldID), the resolvedfieldDescriptionand the qualifiedfieldPath.SchemaDescription.field_path(field_id) -> byteswalksfParentFieldIDup to the top-level field, which names itself as parent, and joins the names with...is forbidden in names, so the result is unambiguous. It returnsbytes, like the names (#127), and raises on an out-of-range ID or a cycle.The schema is built once per call. The
schemaDescriptionproperty itself is unchanged.Tests (
tests/test_rntuple_page_descriptions.py):firstClusterIDs == [0, 5, 9]and first entries 0…900;rntuple/user-class.root, checked against the names root-io-spec'scase.tomluses (fHit.:_0,fHits._0.:_0,fFlavour._0, …);test_extension_columnsgivesint_field,float_field,intvec_field,intvec_field._0;The hardcoded tests gain the new members. Full suite:
370 passed, 53 skipped, 46 xfailed(on top of #127).Breaking changes:
get_extended_page_descriptions()no longer takesincludeSuppressed, and always includes suppressed columns (as empty entries).InterpretablePagehas five new required members.Assisted-by: claude-code:claude-opus-5-5