-
-
Notifications
You must be signed in to change notification settings - Fork 360
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
layouts fixed #2799
layouts fixed #2799
Conversation
@@ -1263,7 +1263,7 @@ function _update_min_padding!(sp::Subplot{PyPlotBackend}) | |||
# optionally add the width of colorbar labels and colorbar to rightpad | |||
if haskey(sp.attr, :cbar_ax) | |||
bb = py_bbox(sp.attr[:cbar_handle]."ax"."get_yticklabels"()) | |||
sp.attr[:cbar_width] = _cbar_width + width(bb) + (sp[:colorbar_title] == "" ? 0px : 30px) | |||
sp.attr[:cbar_width] = width(bb) + (sp[:colorbar_title] == "" ? 0px : 30px) |
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.
_cbar_width
is already contained in the bb
r == 1 ? layout.minpad[2] : 0mm, | ||
c == nc ? layout.minpad[3] : 0mm, | ||
r == nr ? layout.minpad[4] : 0mm | ||
c == 1 ? layout.minpad[1] : pad_left[c], |
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.
My best short explanation:
minimum padding should be propagated to all the children in the grid.
Additionally, perhaps I should add a clarifying comment that minimum_perimeter
actually means minimum_padding
. It took me a while to understand this. Should I?
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.
That definitely looks better than before. Still the heatmaps have a larger gap than the line plots.
This is due to colorbars not having a consistent width nor rules for scaling. Theres always more than actually required padding added for colorbars. I do not think it is related to grid layouting. |
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 work!
This PR has the yet highest
brain_power_used / lines_changed
ratio for me 😄Fixes:
#2783
#2237
#1947
#2695
Perhaps there were other issues reporting some misalignment.
Plots before:
layouts before improvement.pdf
Plots after:
layouts improved.pdf