Skip to content

Include first value when computing automatic TTree::Draw histogram range - #427

Merged
linev merged 2 commits into
root-project:devfrom
auraseb:fix/treedraw-range-skips-first-value
Oct 20, 2026
Merged

linev merged 2 commits into
root-project:devfrom
auraseb:fix/treedraw-range-skips-first-value

Conversation

@auraseb

@auraseb auraseb commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

This Pull request:

Fixes automatic histogram range in TDrawSelector.getMinMaxBins(). The loop which collects min and max values started from index 1, so the first drawn value was ignored. When that value was the smallest or largest one, it landed in the underflow or overflow bin. Affects treeDraw() and rntupleDraw().

Changes:

  1. modules/tree.mjs: start min/max loop from index 0
  2. demo/node/tree_draw_range.js: new test, draws DoubleField and 5 - DoubleField from the existing rntuple_test.root and checks that underflow and overflow are empty; exits with error otherwise
  3. .github/workflows/jsroot-ci.yml: run the new test in all three jobs

Note for jsroot-test: with this change RNTuple/uint32.svg changes. There the first customerId is the minimum, so the histogram now gets the same one bin margin on the left side as on the right side. All other TTree and RNTuple references match.

Fixes:

Values of the first entry lost in underflow or overflow bin when drawing with automatic range.

Fixes #426

@linev linev self-assigned this Oct 20, 2026
@linev

linev commented Oct 20, 2026

Copy link
Copy Markdown
Member

Thanks for the fix!
No idea why I start loop from 1.

Can you split your PR on two commits?
First with bug fix and second with test macro.
Then I can backport fix to older JSROOT versions.

@linev linev added the bug fix label Oct 20, 2026
Signed-off-by: Sebastian Turner <sebastiant3484@gmail.com>
Signed-off-by: Sebastian Turner <sebastiant3484@gmail.com>
@auraseb
auraseb force-pushed the fix/treedraw-range-skips-first-value branch from 1c5df6b to e20c82f Compare October 20, 2026 07:21
@auraseb

auraseb commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Split into two commits: the fix first, then the test macro and its CI lines, both on top of current dev.

About the failing tests_ubuntu jobs: the only diff is RNTuple/uint32.svg in jsroot-test, and it comes from this fix. rntupleDraw uses the same TDrawSelector range code. customerId runs 1000 to 1099, so before the fix the range was [1000, 1101] with no empty bin on the left. Now it is [999, 1101] with one empty bin on each side, as the integer padding intends. All 100 entries are in range in both cases, so only the axis and frame change. The reference can be regenerated with node test.js -c -k RNTuple -o 0.

@linev
linev merged commit 4e85829 into root-project:dev Oct 20, 2026
11 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants