Fix Display output when friend trees are present#8529
Merged
eguiraud merged 4 commits intoroot-project:masterfrom Jun 30, 2021
Merged
Fix Display output when friend trees are present#8529eguiraud merged 4 commits intoroot-project:masterfrom
eguiraud merged 4 commits intoroot-project:masterfrom
Conversation
UpdateList -> InsertBranchName
Before this patch, friend branches or leaves were listed in the output of `GetColumnNames` twice, as `friendname.bname` and as `bname`. Now we only list the longer version. This fixes root-project#8450 ("[DF] Display gets confused by friend columns, prints them twice") as a side-effect.
|
Starting build on |
|
Build failed on ROOT-performance-centos8-multicore/default. Failing tests: |
etejedor
reviewed
Jun 30, 2021
|
|
||
| Friend TTrees with a TTreeIndex are supported from ROOT v6.24. | ||
| Columns coming from the friend trees can be referred to by their full name, like in the example above, | ||
| or the friend tree name can be omitted in case the branch name is not ambiguous (e.g. "MyCol" could be used instead of |
Contributor
There was a problem hiding this comment.
I guess this is already tested elsewhere? The fact that one can refer to a non-ambiguous friend column omitting the friend tree name or not.
Contributor
Author
There was a problem hiding this comment.
yes dataframe_friends.cxx tests it
etejedor
approved these changes
Jun 30, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before this patch, friend branches or leaves were listed in the output of
GetColumnNamestwice, asfriendname.bnameand asbname. Now we only list the longer version.This fixes #8450 as a side-effect.
This PR also adds a test and improves the related docs.
Rationale for the change in behavior of
GetColumnNames: currently, in RDataFrame we have (fairly convoluted) logic to retrieve 3 different lists of branch names given a TTree/TChain:df.GetColumnNames()df.GetColumnNames()plus alternative spellings such asbranchname.leafnamewhenbranchname == leafname, shorthands forfriendname.branchnameas justbranchname, etc.Snapshotby defaultBefore this patch
df.GetColumnNames()returned multiple valid spellings for the same friend branch. That can be confusing, so I'd rather (try to) return only one valid spelling for each available branch/leaf. For consistency withDisplayand to not withhold information from users, among the two valid spellings we always show the "fully qualified friendname.branchname". Users can still use the shorthand "branchname" if it's not ambiguous, as the relevant documentation now points out.I think this solution is the sweet spot between not being surprising (to users and to RDF developers), being easy to implement without further complicating or completely refactoring the branch-retrieval logic and being somewhat backward-compatible.