-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[main] rootls: add flag to show RNTuple field hierarchy #19355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For TTree, rootls
talks about tree
rather than ttree
. Do we want to keep this convention and use ntuple
instead of rntuple
(and with it maybe use -n
instead of -R
)?
I don't have a strong opinion myself, just want to mention it for consideration!
A problem I see with that is that technically we have other "ntuple" classes other than RNTuple (like TNTuple), so maybe we want to be extra specific (it's also the name that people who know it know it by so perhaps more familiar and immediately recognizable) |
Test Results 21 files 21 suites 3d 9h 51m 35s ⏱️ Results for commit 6da3f1c. ♻️ This comment has been updated with latest results. |
Yes, that actually also came up in my mind after posting this comment. It's a good reason to keep it like this, then! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
7ca5ceb
to
b7df1a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I think we should add a roottest
for this.
main/src/rootls.cxx
Outdated
std::size_t maxNameLen = 0, maxTypeLen = 0; | ||
std::vector<const ROOT::RFieldDescriptor *> fields; | ||
fields.reserve(rootField.GetLinkIds().size()); | ||
for (const auto &childId : rootField.GetLinkIds()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have GetFieldIterable
with a comparator parameter. Here would be a use case for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can use GetFieldIterable()
to save a line, yes, but why would I need the comparator?
b7df1a2
to
6da3f1c
Compare
Why isn't both |
@pcanal |
This Pull request:
adds a
-R
flag torootls
to print RNTuple field trees recursively (similarly to the -t flag for TTree). Fields are sorted by name.Sample output (cut for brevity):
Checklist: