-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[WIP] Add RDataSource for TTree #17895
base: master
Are you sure you want to change the base?
Conversation
aa4ea82
to
ffce5d4
Compare
|
Test Results 20 files 20 suites 5d 11h 53m 8s ⏱️ For more details on these failures, see this check. Results for commit 186dbf8. ♻️ This comment has been updated with latest results. |
ffce5d4
to
be68db1
Compare
be68db1
to
6095fe3
Compare
6095fe3
to
ee66417
Compare
b67ad02
to
2336551
Compare
d17de72
to
392845e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a few questions and comments.
One general thing I noticed is that close to none of the functions have documentation. It can probably be retrofitted, but it also stopped me from reviewing what's being done.
A new RDataSource derived class is implemented for TTree data processing. The class follows almost entirely the same approach for processing of other data sources, with the exception of the MT case which still needs to be partially handled externally by TTreeProcessorMT. Nonetheless, the class and its API can be used throughout the RDF codebase to centralise all TTree-related usage and processing. While implementing the class, a few missing features or limitations of RDataSource were found which needed API extensions. For the moment, all the API extensions of RDataSource are private and it will be later decided whether to make them public or not.
392845e
to
5995528
Compare
The rest of the RDF codebase is adapted to use the new TTree data source. All the previous usage of raw TTree classes or API is now delegated to the RDataSource. With this commit, there are no more ways to create an RDataFrame processing a TTree without an RDataSource.
5995528
to
186dbf8
Compare
All tests are working on my machine with these changes, so I want to start testing on all platforms. Changes will be later split in multiple commits/PRs.