Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vepadulano
Copy link
Member

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.

@vepadulano
Copy link
Member Author

vepadulano commented Mar 5, 2025

Sibling roottest PR at root-project/roottest#1270 roottest tests have already been taken care of

Copy link

github-actions bot commented Mar 6, 2025

Test Results

    20 files      20 suites   5d 11h 53m 8s ⏱️
 2 726 tests  2 724 ✅ 0 💤 2 ❌
52 591 runs  52 588 ✅ 0 💤 3 ❌

For more details on these failures, see this check.

Results for commit 186dbf8.

♻️ This comment has been updated with latest results.

@vepadulano vepadulano marked this pull request as ready for review March 19, 2025 08:19
@vepadulano vepadulano requested review from dpiparo and hageboeck March 19, 2025 08:19
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.

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.
@vepadulano vepadulano force-pushed the rdf-ttree-datasource branch from 392845e to 5995528 Compare March 20, 2025 11:36
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.
@vepadulano vepadulano force-pushed the rdf-ttree-datasource branch from 5995528 to 186dbf8 Compare March 20, 2025 14:45
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.

2 participants