Skip to content

[df] Handle containers in RHist* filling - #22899

Merged
hahnjo merged 3 commits into
root-project:masterfrom
hahnjo:hist-df-containers
Aug 5, 2026
Merged

hahnjo merged 3 commits into
root-project:masterfrom
hahnjo:hist-df-containers

Conversation

@hahnjo

@hahnjo hahnjo commented Jul 23, 2026

Copy link
Copy Markdown
Member

As for the existing Histo actions, the sizes of all containers must match while scalars are broadcasted. As an extension, this works recursively, so nested vectors are handled one level at a time.

@github-actions

github-actions Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 16h 13m 55s ⏱️
 3 853 tests  3 853 ✅ 0 💤 0 ❌
79 408 runs  79 408 ✅ 0 💤 0 ❌

Results for commit cfe7102.

♻️ This comment has been updated with latest results.

@gpetruc

gpetruc commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Hi!

Thanks, this feature is what I was looking for when trying to switch my code from Histo to Hist.
Would it cover also the case of a vector column and a scalar weight (to be broadcasted)?

Giovanni

@hahnjo

hahnjo commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

Hi @gpetruc

Thanks, this feature is what I was looking for when trying to switch my code from Histo to Hist. Would it cover also the case of a vector column and a scalar weight (to be broadcasted)?

Yes, the support should be a superset of what Histo can do, notably for nested collections. Weight columns will follow the same rules as value columns wrt broadcasting.

@hahnjo
hahnjo force-pushed the hist-df-containers branch from 613e0bd to ecef742 Compare July 28, 2026 14:29
@hahnjo hahnjo changed the title [df] Handle containers in RHist filling [df] Handle containers in RHist* filling Jul 28, 2026
@hahnjo
hahnjo marked this pull request as ready for review July 28, 2026 14:29
@hahnjo
hahnjo requested a review from hageboeck as a code owner July 28, 2026 14:29

@hageboeck hageboeck left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. I think we should have added an include in the past, maybe that's a good opportunity to add it?

Comment thread tree/dataframe/inc/ROOT/RDF/ActionHelpers.hxx
@hahnjo
hahnjo force-pushed the hist-df-containers branch from ecef742 to 31794b1 Compare August 4, 2026 07:36

@vepadulano vepadulano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you so much for this nice improvement!

Comment thread tree/dataframe/inc/ROOT/RDF/RInterface.hxx Outdated
hahnjo added 3 commits August 4, 2026 11:52
It abstracts the loop when Exec'uting with data containers, in the
following commit for RHist*FillHelper and in the future for the other
FillHelpers.
As for the existing Histo actions, the sizes of all containers must
match while scalars are broadcasted. As an extension, this works
recursively, so nested vectors are handled one level at a time.
This reduces repeated arguments that must be consistent.
@hahnjo
hahnjo force-pushed the hist-df-containers branch from 31794b1 to cfe7102 Compare August 4, 2026 10:01
@hahnjo
hahnjo merged commit 50cad9d into root-project:master Aug 5, 2026
34 checks passed
@hahnjo
hahnjo deleted the hist-df-containers branch August 5, 2026 06:16
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.

4 participants