Skip to content

Support RNTuple as possible output format #1026

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

Merged
merged 1 commit into from
Apr 14, 2025
Merged

Support RNTuple as possible output format #1026

merged 1 commit into from
Apr 14, 2025

Conversation

ponyisi
Copy link
Collaborator

@ponyisi ponyisi commented Apr 5, 2025

  • Support RNTuple as output format for the uproot-raw and func_adl_uproot transformers
    • Add necessary enums; update uproot-raw transformer
  • uproot 5.6 doesn't yet implement compression when writing RNTuple, so this option is not yet recommended for production.

@ponyisi
Copy link
Collaborator Author

ponyisi commented Apr 5, 2025

Good to go here I think for RNTuple output (see #999). I do not recommend rntuple yet (uproot does not yet have compression enabled for the output in 5.6.0) but the code paths technically work and produce correct output.

@ponyisi ponyisi requested a review from Copilot April 6, 2025 14:39
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • code_generator_raw_uproot/transformer_capabilities.json: Language not supported
Comments suppressed due to low confidence (2)

code_generator_raw_uproot/servicex/templates/transform_single_file.py:29

  • In the root-file branch, the previous conditional that checked if v[0].fields existed was removed. Consider reinstating or handling the case where v[0].fields might be empty to avoid potential runtime errors.
if output_format in ('root-file', 'root-rntuple'):

code_generator_raw_uproot/servicex/raw_uproot_code_generator/request_translator.py:150

  • The change from constructing a dict of field types to directly returning arr.layout may affect consumers expecting a specific structure. Ensure that downstream code is compatible with this new format.
rv_arrays_trees[outtreename] = (None, arr.layout)

@gordonwatts gordonwatts changed the title RNTuple support, first version Support RNTuple as possible output format Apr 7, 2025
Copy link
Collaborator

@gordonwatts gordonwatts left a comment

Choose a reason for hiding this comment

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

Huh - copilot is now reviewing our PRs!

Ok - this looks fine - My only comments are style comments (nested if statements, etc.). I also cleaned up the PR text because it is often used to generate change logs. I'll mark approve, but you might made modifications regardless.

@ponyisi
Copy link
Collaborator Author

ponyisi commented Apr 11, 2025

I should note that the next uproot release will remove the need for funny ListArray manipulation and will compress the RNTuples, so it makes sense to hold off on this PR until we get the new version. (The code here should still work with the new release but will have code branches that are no longer relevant.)

Copy link
Contributor

@BenGalewsky BenGalewsky left a comment

Choose a reason for hiding this comment

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

Excellent

@BenGalewsky BenGalewsky merged commit 3bf3c9b into develop Apr 14, 2025
69 checks passed
@BenGalewsky BenGalewsky deleted the rntuple branch April 14, 2025 14:42
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.

3 participants