Skip to content

fix: give unweighted histogram bins of negative content a variance - #1826

Merged
ariostas merged 2 commits into
scikit-hep:mainfrom
ariostas:fix/histograms
Nov 10, 2026
Merged

ariostas merged 2 commits into
scikit-hep:mainfrom
ariostas:fix/histograms

Conversation

@ariostas

@ariostas ariostas commented Nov 6, 2026 •

Copy link
Copy Markdown
Member

🤖 AI text below 🤖

Part of the split of #1814 (the list is in #1834).

  • Variances of unweighted bins: without fSumw2, the variance of a TH1/TH2/TH3 bin is now the absolute value of its content, like ROOT's TH1::GetBinError (sqrt(|content|)). Before, it was 0 for bins of negative content. Integer contents are converted to floats first, so the lowest integer of a TH1C/S/I doesn't overflow.
  • Comparing profiles: == on two TProfiles raised AttributeError, because Histogram.__eq__ calls _values_variances, which profiles didn't have. Profiles now have it.

tests/test_1826_fix_histograms.py holds the regression tests: every test in it fails on main (for parametrized tests, at least one of their cases).

This changes what variances() and errors() return for unweighted histograms with negative bins, and what == compares, which is why it's a separate PR. to_boost() and to_hist() are unaffected, since they only take variances from weighted histograms.

@github-actions github-actions Bot added the type/fix PR title type: fix (set automatically) label Nov 6, 2026
@ariostas
ariostas marked this pull request as ready for review November 6, 2026 14:47
@codecov

codecov Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.42%. Comparing base (b4acc0f) to head (a4b4923).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
Files with missing lines Coverage Δ
src/uproot/behaviors/TH1.py 87.75% <100.00%> (+4.64%) ⬆️
src/uproot/behaviors/TH2.py 94.23% <100.00%> (-0.11%) ⬇️
src/uproot/behaviors/TH3.py 94.64% <100.00%> (+15.69%) ⬆️
src/uproot/behaviors/TProfile.py 90.55% <100.00%> (+4.67%) ⬆️

... and 5 files with indirect coverage changes

Like ROOT's TH1::GetBinError, the variance of a bin without fSumw2 is the
absolute value of its content, rather than 0 when the content is negative.
TProfile gets _values_variances, which the histogram conversions use. Found
by the tests of scikit-hep#1814.

Assisted-by: Claude Code:claude-opus-5-5
numpy.abs keeps the dtype, and the absolute value of the lowest integer of a
dtype doesn't fit in it, so a TH1C bin of -128 had a variance of -128 (and
TH1S, TH1I, TH2 and TH3 likewise).

Assisted-by: Claude Code:claude-opus-5-5

@ianna ianna 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.

@ariostas -- Thanks! LGTM.

@ariostas
ariostas merged commit 4885ed4 into scikit-hep:main Nov 10, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/fix PR title type: fix (set automatically)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants