Skip to content

[tree] Clear error when calling CopyTree on a chain with friends#21260

Open
dpiparo wants to merge 1 commit intoroot-project:masterfrom
dpiparo:msg_for_ROOT-10778
Open

[tree] Clear error when calling CopyTree on a chain with friends#21260
dpiparo wants to merge 1 commit intoroot-project:masterfrom
dpiparo:msg_for_ROOT-10778

Conversation

@dpiparo
Copy link
Member

@dpiparo dpiparo commented Feb 12, 2026

Addresses ROOT-10778 by printing a clear, meaningful message about an unsupported operation being carried out.

@dpiparo dpiparo self-assigned this Feb 12, 2026
@dpiparo dpiparo marked this pull request as ready for review February 12, 2026 13:20
@dpiparo dpiparo requested a review from pcanal as a code owner February 12, 2026 13:20
Copy link
Member

@hageboeck hageboeck left a comment

Choose a reason for hiding this comment

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

Could you also add the "known limitation" as a paragraph at the end of the doxygen docstring?

Such as:

/// <b> Known limitations </b>: For a TChain with friends ...

@github-actions
Copy link

github-actions bot commented Feb 12, 2026

Test Results

    20 files      20 suites   2d 21h 55m 20s ⏱️
 3 769 tests  3 769 ✅ 0 💤 0 ❌
68 453 runs  68 453 ✅ 0 💤 0 ❌

Results for commit ec1dfb8.

♻️ This comment has been updated with latest results.

@hageboeck
Copy link
Member

This seems to render even better:

/// \par Known limitations
/// When friend trees are in use, ...
///
/// Other text

See preview here.

@dpiparo
Copy link
Member Author

dpiparo commented Feb 12, 2026

Thanks for the suggestions: all implemented.

Copy link
Member

@hageboeck hageboeck left a comment

Choose a reason for hiding this comment

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

Maybe one extra change.

/// NOTE that only the active branches are copied.
///
/// ### Known limitations
/// - This method is not supported if the TChain instance has friends
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// - This method is not supported if the TChain instance has friends
/// - This method is not supported if used on a TChain instance with friends

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea. I will add it once the jobs succeed after flagging this PR [skip-ci] to conserve a bit the resources of the CI.

TTree* TTree::CopyTree(const char* selection, Option_t* option /* = 0 */, Long64_t nentries /* = TTree::kMaxEntries */, Long64_t firstentry /* = 0 */)
{
// A clear error for ROOT-10778
if (GetListOfFriends() && 0 == strcmp(this->ClassName(), "TChain")) {
Copy link
Member

@pcanal pcanal Feb 13, 2026

Choose a reason for hiding this comment

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

Suggested change
if (GetListOfFriends() && 0 == strcmp(this->ClassName(), "TChain")) {
if (GetListOfFriends() && IsA()->InheritsFrom("TChain")) {

or 'better yet' move this code into a (new) TChain::CopyTree.

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

Comments