Skip to content

Conversation

@siliataider
Copy link
Contributor

@siliataider siliataider commented Jun 4, 2025

This Pull request:

Changes or fixes:

This PR extends the functionality of the RNTupleImporter to allow creating ntuples within directories in the destination file during the conversion.

auto importer = RNTupleImporter::Create(treeFileName, "Tuple/DecayTree", ntupleFileName);

-> This would create the directory Tuple if it doesn't already exists in the destination file and place the ntuple DecayTree inside it.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #18965

@siliataider siliataider requested a review from jblomer as a code owner June 4, 2025 16:29
@siliataider siliataider requested review from enirolf and silverweed and removed request for jblomer June 4, 2025 16:30
@siliataider siliataider self-assigned this Jun 4, 2025
@siliataider siliataider requested a review from jblomer June 4, 2025 16:30
@github-actions
Copy link

github-actions bot commented Jun 4, 2025

Test Results

    19 files      19 suites   3d 9h 43m 42s ⏱️
 2 813 tests  2 813 ✅ 0 💤 0 ❌
51 909 runs  51 909 ✅ 0 💤 0 ❌

Results for commit 351935f.

♻️ This comment has been updated with latest results.

@dpiparo
Copy link
Member

dpiparo commented Jun 5, 2025

thanks for these changes. Is it easy to add a test?

@dpiparo
Copy link
Member

dpiparo commented Jun 5, 2025

very nice. Thanks.

@siliataider siliataider force-pushed the importer branch 4 times, most recently from a9a87b3 to 3a69a29 Compare June 5, 2025 13:35
@siliataider siliataider requested a review from silverweed June 5, 2025 14:32
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! Maybe wait for @enirolf's approval since she's the author of the RNTupleImporter

Copy link
Contributor

@enirolf enirolf left a comment

Choose a reason for hiding this comment

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

Not the author but I can still give my review :). Functionality looks good! I think it could benefit from some documentation but after that should be good to go.

Copy link
Contributor

@enirolf enirolf left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@siliataider siliataider merged commit 7a0dd4f into root-project:master Jun 17, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support creating ntuples inside a directory with the RNTupleImporter

4 participants