Skip to content

[hist] Implement initial RProfile#22588

Draft
hahnjo wants to merge 7 commits into
root-project:masterfrom
hahnjo:hist-profile
Draft

[hist] Implement initial RProfile#22588
hahnjo wants to merge 7 commits into
root-project:masterfrom
hahnjo:hist-profile

Conversation

@hahnjo

@hahnjo hahnjo commented Jun 12, 2026

Copy link
Copy Markdown
Member

It builds on RHistEngine with an additional variable.

@hahnjo hahnjo self-assigned this Jun 12, 2026
@hahnjo hahnjo added in:Hist test coverage Run the test code coverage workflow on this PR labels Jun 12, 2026
\warning This is part of the %ROOT 7 prototype! It will change without notice. It might trigger earthquakes.
Feedback is welcome!
*/
class RProfile final {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Imho this should be called something more specific to histograms (RHistProfile? RProfileHist?), as "profile" is a generic term that could be overloaded (e.g. a timing profile) and since it's directly in the ROOT namespace we might want to avoid that confusion.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

hm, I think the term "profile" is well established in HEP to not be confusing in the context of a data analysis framework. For example, historically the class is called TProfile, Boost.Histogram calls it make_profile, and for YODA it's BinnedProfile / Profile1D etc.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm more concerned about ambiguities in internal usage by developers than about confusion from users (RProfile might be used as an internal class in some other context). Nothing that some namespaces can't solve of course, but might be worth thinking about.

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown

Test Results

    20 files      20 suites   3d 4h 10m 6s ⏱️
 3 867 tests  3 867 ✅ 0 💤 0 ❌
69 164 runs  69 164 ✅ 0 💤 0 ❌

Results for commit ce6f93e.

♻️ This comment has been updated with latest results.

hahnjo added 4 commits June 15, 2026 14:43
It builds on RHistEngine with an additional variable.
SetBinContent, Add, AddAtomic, Clear, Clone, Scale; implemented
similarly to RHist, forwarding to RHistEngine and RHistStats.
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.23077% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 59.77%. Comparing base (7548ce3) to head (ce6f93e).
⚠️ Report is 83 commits behind head on master.

Files with missing lines Patch % Lines
hist/histv7/inc/ROOT/RProfile.hxx 99.14% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #22588      +/-   ##
==========================================
+ Coverage   59.75%   59.77%   +0.02%     
==========================================
  Files        3367     3368       +1     
  Lines      494970   495515     +545     
  Branches    95206    95270      +64     
==========================================
+ Hits       295749   296187     +438     
- Misses     199221   199328     +107     
Flag Coverage Δ
unittests 59.77% <99.23%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in:Hist test coverage Run the test code coverage workflow on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants