Skip to content

Conversation

Krmjn09
Copy link
Collaborator

@Krmjn09 Krmjn09 commented Aug 6, 2025

The testing if the logic is still pending because I am unable to produce root file for index and splitindex columns

@Krmjn09 Krmjn09 requested review from linev and silverweed August 6, 2025 06:11
Copy link
Contributor

@silverweed silverweed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I left a couple of comments.

Also, to produce columns of type Index64/SplitIndex64 you can just create Strings. If the RNTuple is compressed (default), the principal column of a String will be of type SplitIndex64; if you want a non-split column you can create another string field like this:

  auto nonSplitString = std::make_unique<RField<std::string>>("nonSplit");
  nonSplitString->SetColumnRepresentatives({{ENTupleColumnType::kIndex64, ENTupleColumnType::kChar}});
  model->AddField(std::move(nonSplitString));

We don't have a way to automatically create fields with (Split)Index32 types, but you can do the same thing described above and replace kIndex64 with the 32 versions, so you end up with an RNTuple with 4 strings, each using a different column type for its indices.

@Krmjn09 Krmjn09 requested a review from silverweed August 6, 2025 17:45
Copy link
Collaborator Author

@Krmjn09 Krmjn09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After testing the code now on strings.root file I can successfully see the values for all 4 types of columns as you can see below

image image image image

Copy link
Contributor

@silverweed silverweed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good! Next step will be decoding the zigzag-encoded SplitInt column types.
Also, please open a new PR where you add some testing of this string decoding inside rntuple_test.js (you can add the strings.root RNTuple in the repo for that)

@silverweed silverweed merged commit d8322f2 into root-project:dev Aug 7, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants