[ntuple] Add support for attributes reading#21778
[ntuple] Add support for attributes reading#21778silverweed wants to merge 8 commits intoroot-project:masterfrom
Conversation
Test Results 21 files 21 suites 3d 4h 16m 37s ⏱️ Results for commit e1b0b9f. ♻️ This comment has been updated with latest results. |
silverweed
force-pushed
the
ntuple_attr_read
branch
4 times, most recently
from
487b964 to
5da9754
Compare
silverweed
force-pushed
the
ntuple_attr_read
branch
3 times, most recently
from
d68c286 to
28e4497
Compare
| for (const auto *field : userFieldRoot->GetConstSubfields()) { | ||
| fUserModel->AddField(field->Clone(field->GetFieldName())); | ||
| } |
There was a problem hiding this comment.
We may want to construct the fields directly from the constructor: When calling RNTupleReader::GetModel to reconstruct the full model, it will register all fields / columns and prefetch + decompress all pages. That's probably not what we want with the views below.
| } | ||
|
|
||
| ROOT::Experimental::RNTupleAttrEntryIterable::RIterator::Iter_t | ||
| ROOT::Experimental::RNTupleAttrEntryIterable::RIterator::Next() const |
There was a problem hiding this comment.
I'm not sure about the name since, without a filter, Next() may not actually advance to the next attribute. Can ++fCur be moved here?
There was a problem hiding this comment.
We could, but I'm wondering how to set the initial value of fCur in the constructor if we do...
There was a problem hiding this comment.
Ah, right... So in the end, it's something like SkipFiltered() though with that name I would more expect it to modify fCur directly
There was a problem hiding this comment.
Yeah. I can definitely change the name to make it more explicit.
silverweed
force-pushed
the
ntuple_attr_read
branch
from
28e4497 to
d807009
Compare
This makes it easier to keep the meta fields names and indices in sync and to automatically define and verify their count
silverweed
force-pushed
the
ntuple_attr_read
branch
from
d807009 to
e1b0b9f
Compare
silverweed
force-pushed
the
ntuple_attr_read
branch
from
e1b0b9f to
8ed4073
Compare
| // Initialize user model | ||
| fUserModel = RNTupleModel::Create(); | ||
|
|
||
| // Validate meta model format |
There was a problem hiding this comment.
Are we actually checking the schema version major and minor somewhere? Maybe I'm missing it in another place...
There was a problem hiding this comment.
Not really for now. I guess we want to do the check in the Serializer, but probably in a way that if the version differs, reading the whole RNTuple doesn't fail and it only fails if you actually try to open the attribute set.
silverweed
force-pushed
the
ntuple_attr_read
branch
from
8ed4073 to
2cd935d
Compare
silverweed
force-pushed
the
ntuple_attr_read
branch
from
2cd935d to
9bbb92d
Compare
2 participants
Adds support for RNTuple attribute reading in the form of the
ROOT::Experimental::RNTupleAttrSetReaderclass plus some ancillary classes.Main features added:
RNTupleReader::OpenAttributeSet-> returns an RNTupleAttrSetReader for a specific attribute setRNTupleAttrSetReader::GetAttributes*-> various methods that return iterables over attribute entry indicesRNTupleAttrSetReader::LoadEntry(index[, entry])-> loadsindex-th attribute entry (similar toRNTupleReader::LoadEntry). Meant to be used withRNTupleAttrSetReader::GetAttributes*See the tests for basic usage, e.g.
BasicReadingWritinginntuple_attributes.cxx