-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[hist] Implement AutoZoom of TH1/TH2/TH3/TAxis to filled range #18459
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
Features was implemented long-long time ago in JSROOT,
now it is time to have it in ROOT.
But several changes are required:
- exclude loop repetition in
TH1::GetRangeOfFilledWeights
- if
TH1::AutoZoom
is invoked, it only should affect that histogram and not change zooming in all other histograms in the pad - same true for
TAxis::AutoZoom
- it only should change properties of this axis and not all histograms in the pad - but must be checked with superimposed histograms, THStack, TMultiGraph - they all can have side effects
- fix failures with roottest
Test Results 18 files 18 suites 4d 0h 0m 19s ⏱️ Results for commit d4400a6. ♻️ This comment has been updated with latest results. |
This is already the behavior 'most of the time'. Only if DrawFrame was called before, the multi-analysis is run. |
|
as pointed out by linev
derived class as found out by linev
We need to wait with this PR until |
I thought it was already merged, and I had rebased and synced into one. |
I tried your code and have several more comments.
See my simple demo. With With superimposed histograms |
I guess that's a feature of "TH1::UnZoom" that is also not nice, that could be handled in a separate PR ?
I have implemented this now. It could be modified to accept a relative percentage rather than a fixed number of bins if wanted.
I do not understand what you mean. I am proposing AutoZoom to show all peaks, rather than the most significant.
I kind of disagree, sometimes I like to zoom in to see a specific thing, and then AutoZoom to zoom out to the filled range. Maybe ZoomToFit is a better word for the function??
I had made a mistake. Is now fixed, thanks for finding out. You need though to click on the x-axis and click on AutoZoomAll, not AutoZoom. An equivalent function can be created in the TH1::AutoZoomAll if you think it's worth.
Support has been added now, via TAxis::AutozoomAll. |
Still, AutoZoom on simple TH1 with few entries changes Y-zoom, |
But Unzoom does the same, it's changing that too. It's zooming in rather than out.
Yes, but I argue that that's intended. Autozoom in matplotlib works also like that. |
Then one need to fix
Use-case is simple. One makes approx zooming with mouse and then call method to get peak fully displayed. |
thanks to linev
Default drawing histogram option is to leave a YMARGIN range of 10% in YMAX. So if you call Unzoom, it was calling GetMaximum, which calculated the exact maximum, and then called SetMaximum, thus overwriting the stored value. Use instead GetMaximumStored to prevent recalculation if not set by user.
Last commit tries to fix that. |
This Pull request:
Changes or fixes:
Fixes #14538
Checklist: