[TTree] Enable importing all (filled) branches from all the files, not just the first one#17650

Open

ferdymercury wants to merge 1 commit intoroot-project:masterfrom

ferdymercury:mergebranch

Collaborator

ferdymercury commented Feb 6, 2025 •

edited

Loading

This is a new feature, only turned on if selected.

This relates to #14558 by @krasznaa
and maybe #12510 by @MoAly98 and @rankunlin

and also: https://root-forum.cern.ch/t/hadd-not-working-when-missing-branches-in-one-of-the-root-files/62957

Checklist:

tested changes locally
updated the docs (if necessary)

ferdymercury requested a review from pcanal as a code owner

February 6, 2025 16:42

ferdymercury requested a review from silverweed

February 6, 2025 16:44

github-actions bot commented Feb 6, 2025 •

edited

Loading

Test Results

22 files 22 suites 3d 13h 37m 35s ⏱️
3 692 tests 3 687 ✅ 0 💤 5 ❌
79 256 runs 79 250 ✅ 0 💤 6 ❌

For more details on these failures, see this check.

Results for commit 102e2cb.

♻️ This comment has been updated with latest results.

Contributor

krasznaa commented Feb 6, 2025

I believe I was wrongfully accused for #14558. 😛 But happy to receive any bugfixes for ATLAS too. 😄

Collaborator Author

ferdymercury commented Feb 6, 2025 •

edited

Loading

I believe I was wrongfully accused for #14558. 😛 But happy to receive any bugfixes for ATLAS too. 😄

I thought you reported https://its.cern.ch/jira/browse/ROOT-4716 (which was moved to 14558), but anyway it doesn't matter hehe.

Contributor

krasznaa commented Feb 6, 2025

I believe I was wrongfully accused for #14558. 😛 But happy to receive any bugfixes for ATLAS too. 😄

I thought you reported https://its.cern.ch/jira/browse/ROOT-4716 (which was moved to 14558), but anyway it doesn't matter hehe.

😛 I just looked at the author of that issue here on GitHub, didn't read it properly.

Yes, you were correct, I was wrong. 😄

pcanal requested changes

View reviewed changes

Member

pcanal left a comment

See comments.

tree/tree/src/TTree.cxx Outdated Show resolved Hide resolved

tree/tree/src/TTree.cxx Outdated Show resolved Hide resolved

tree/tree/src/TTree.cxx Outdated Show resolved Hide resolved

io/io/test/TFileMergerTests.cxx Outdated Show resolved Hide resolved

ferdymercury commented

View reviewed changes

tree/tree/src/TTree.cxx Outdated Show resolved Hide resolved

ferdymercury mentioned this pull request

[TTree] when merging, ignore trees without branches #17652

Merged

ferdymercury commented

View reviewed changes

io/io/test/TFileMergerTests.cxx Outdated Show resolved Hide resolved

ferdymercury commented

View reviewed changes

io/io/test/TFileMergerTests.cxx Outdated Show resolved Hide resolved

pcanal reviewed

View reviewed changes

tree/tree/src/TTree.cxx Outdated

                     Long64_t nentries = tree->GetEntries();
                     if (nentries == 0) continue;
+                    ImportBranches(tree);

Member

pcanal Feb 6, 2025

This is a significant new feature and should probably not be enabled by default. In addition (compare to RNTuple which has a similar feature) we do not offer a clear way to select the value used for backfillin. (It is not clear if the current code use a 'default' value, the first value from the new tree or a random value).

ferdymercury commented

View reviewed changes

io/io/test/TFileMergerTests.cxx Outdated Show resolved Hide resolved

ferdymercury commented

View reviewed changes

tree/tree/src/TTree.cxx Show resolved Hide resolved

ferdymercury marked this pull request as draft

February 7, 2025 08:04

ferdymercury commented

View reviewed changes

tree/tree/src/TTree.cxx Outdated Show resolved Hide resolved

tree/tree/src/TTree.cxx Outdated Show resolved Hide resolved

tree/tree/src/TTree.cxx Outdated Show resolved Hide resolved

ferdymercury requested a review from pcanal

February 7, 2025 17:14

ferdymercury commented

View reviewed changes

io/io/test/TFileMergerTests.cxx Outdated Show resolved Hide resolved

tree/tree/src/TTree.cxx Show resolved Hide resolved

pcanal changed the title ~~[TTree] merge also branches when merging TTrees~~ [TTree] Enable merging all the branches from all the files

ferdymercury commented

View reviewed changes

io/io/test/TFileMergerTests.cxx Outdated Show resolved Hide resolved

ferdymercury marked this pull request as ready for review

February 8, 2025 16:01

dpiparo assigned silverweed

silverweed assigned pcanal

ferdymercury force-pushed the mergebranch branch 2 times, most recently from 72e1fa0 to afa323e Compare

October 20, 2025 16:10

ferdymercury changed the title ~~[TTree] Enable merging all the branches from all the files~~ [TTree] Enable importing all the branches from all the files

ferdymercury force-pushed the mergebranch branch 3 times, most recently from ec48ba4 to 780d328 Compare

October 20, 2025 17:18

ferdymercury requested a review from vepadulano

October 20, 2025 17:20

ferdymercury changed the title ~~[TTree] Enable importing all the branches from all the files~~ [TTree] Enable importing all (filled) branches from all the files, not just the first one

ferdymercury force-pushed the mergebranch branch from 780d328 to e4b19ea Compare

October 20, 2025 17:25


          [TTree] Add option to import branches not present in first tree when …

102e2cb

…merging TTrees

Fixes root-project#14558
Fixes root-project#12510

ferdymercury force-pushed the mergebranch branch from 0374625 to 102e2cb Compare

October 21, 2025 07:12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet