Skip to content

Deflate axis#8760

Merged
couet merged 4 commits intoroot-project:masterfrom
couet:deflate-axis
Aug 20, 2021
Merged

Deflate axis#8760
couet merged 4 commits intoroot-project:masterfrom
couet:deflate-axis

Conversation

@couet
Copy link
Copy Markdown
Member

@couet couet commented Jul 28, 2021

fix #8598
It is reasonable to call automatically LabelsDeflate at drawing time when plotting histogram with labels.

@couet couet requested a review from lmoneta July 28, 2021 07:46
@couet couet self-assigned this Jul 28, 2021
@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Jul 28, 2021

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Jul 28, 2021

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft11.dyndns.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

lmoneta
lmoneta requested changes Aug 3, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

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

Thank you Olivier for the PR.
Checking on the fact that an axis can be extended and is alphanumeric is the correct thing, because you can have histograms with labels and the users explicitly does not want the axis to be inflated/deflated.

hist/histpainter/src/THistPainter.cxx
return;
}

// Deflate the labels in case of alphanumeric labels
Copy link
Copy Markdown
Member

@lmoneta lmoneta Aug 3, 2021

Choose a reason for hiding this comment

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

The correct condition is:

if (fXaxis.CanExtend() && fXaxis.IsAlphanumeric()) fH->LabelsDeflate("X");
if (fYaxis.CanExtend() && fYaxis.IsAlphanumeric()) fH->LabelsDeflate("Y");

and should we add also the case for the Z axis (for TH3 objects) ?

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Aug 19, 2021

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@couet
Copy link
Copy Markdown
Member Author

couet commented Aug 19, 2021

@lmoneta
Hello Lorenzo, I implemented your suggestions. I will merge as soon as you approve it.

lmoneta
lmoneta approved these changes Aug 20, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

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

LGTM now !
Thank you Olivier for the update !

@couet couet merged commit 5581794 into root-project:master Aug 20, 2021
@couet couet deleted the deflate-axis branch August 20, 2021 08:18
pzhristov pushed a commit to alisw/root that referenced this pull request Aug 27, 2021
@couet
Deflate axis (root-project#8760)
2dc16bf 
* [skip-ci] images are not generated foe these tutorials

* Deflate axis at painting time

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

Reviewers

@lmoneta lmoneta lmoneta approved these changes

Assignees

@couet couet

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Call automatically LabelsDeflate at drawing time

3 participants

@couet @phsft-bot @lmoneta