Skip to content

get_tightbbox doesn't account for markeredgewidth #16606

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

Open
brunobeltran opened this issue Feb 28, 2020 · 10 comments · May be fixed by #17199 or #17198
Open

get_tightbbox doesn't account for markeredgewidth #16606

brunobeltran opened this issue Feb 28, 2020 · 10 comments · May be fixed by #17199 or #17198
Labels
keep Items to be ignored by the “Stale” Github Action

Comments

@brunobeltran
Copy link
Contributor

brunobeltran commented Feb 28, 2020

Simple to solve problem, making issue just to refer to it in the PR.

Bug report

For lines, Axes.get_tightbbox relies on Line2D.get_window_extent, which computes extra padding due to markers as

ms = (self._markersize / 72.0 * self.figure.dpi) * 0.5

but this can cut off half of the marker edge.

This phenomena which is only visible if you have a line that extends outside of a plot, and has markeredgewidth > 1. For simple markers, (like 'S', 'O'), the code should instead read

extra_pts = self._markersize + self._markeredgewidth
ms = (extra_pts / 72.0 * self.figure.dpi) * 0.5

I'll edit this shortly to include code that reproduces the bug, after I document for all the common markers what the expected behavior should be.

Matplotlib version

  • Operating system: Linux
  • Matplotlib version: 3.2.0rc2.post1594.dev0+gead45c7e6
  • Matplotlib backend: module://ipykernel.pylab.backend_inline`
  • Python version: 3.7.3
    MPL installed from Github, Python is miniconda.
@brunobeltran
Copy link
Contributor Author

brunobeltran commented Mar 5, 2020

This seems like a pretty niche use case, and begs the question what I'm doing to make it necessary:

Say you're making a figure for publication, so that the font size, and figure width is fixed. In modern biology, it's not uncommon to be VERY short on figure space, leading to figures with funny sizes, like the following:

fig, axs = plt.subplots(2, figsize=(2,4))
for ax in axs:
    for dash, marker in zip(['-', '-.', '--'], ['D', 'd', 'o']):
        N = 5
        x = np.random.rand(N)
        y = np.random.rand(N)
        ax.plot(x, y, 
                 marker=marker, markersize=12, markeredgewidth=5,
                 fillstyle='full', markerfacecolor='white', 
                 linestyle=dash, lw=3, clip_on=False)

example_overlap

As you can see, the markers overlap strongly across plots. I can easily fix this by just spacing the axes myself. But for that I need to know the ax.get_tightbbox, which is currently wrong.

Normally being a little bit off wouldn't be a big deal, but in these figures where the marker edge width is comparable to the figure size, (say >1% of total figure size or so) then being off by a bit can make some of the axes look noticeably misspaced.

I like to think I understand the internals of matplotlib more than the average use, and even when being pretty clever, I simply don't have a way to space the axes nicely without knowing where the bbox's are

from bruno_util.plotting import make_at_aspect
def random_large_markers(ax, N=5):
    for dash, marker in zip(['-', '-.', '--'], ['D', 'd', 'o']):
        ax.plot(np.random.rand(N), np.random.rand(N), 
                marker=marker, markersize=12, markeredgewidth=5,
                fillstyle='full', markerfacecolor='white', 
                linestyle=dash, lw=3, clip_on=False)
    ax.set_xlim([0, 1])
    ax.set_ylim([0, 1])
    ax.set_xlabel('x axis')
    ax.set_ylabel('y axis')

golden_ratio = (1 + np.sqrt(5))/2
col_width = 2 # inches. imposed by journal format
fig = make_at_aspect([random_large_markers, random_large_markers], 
                     [1/golden_ratio, 1/golden_ratio], col_width)

example_overlap_stack

@brunobeltran
Copy link
Contributor Author

Originally I had hoped to just add markeredgewidth/2 padding to the existing bbox calculation code, but since this underestimates the extents of corners, if I'm not careful I can cause my markers to be cut off (which looks V ugly).

test

@anntzer
Copy link
Contributor

anntzer commented Mar 5, 2020

What about just doing +markeredgewidth instead?

@brunobeltran
Copy link
Contributor Author

brunobeltran commented Mar 5, 2020

I'm sure you already know this, but that would be an overestimate for some markers (e.g. 'o', 's', etc.), where markeredgewidth/2 is the correct amount.

So using the code in my PR would allow the figure boundary to be set "exactly flush" with the markers, instead of leaving annoying whitespace that has to be removed in Illustrator.

But if it is agreed that an overestimate is what we want (even though I already have the code to compute the bbox exactly written), then I can compute what the correct amount to add is (assuming e.g. the miter default limit). I'd guess that it will be a hair larger than +markeredgewidth, because of some sharp corners like the top of marker='d', but you might be right, I'd have to check.

@anntzer
Copy link
Contributor

anntzer commented Mar 5, 2020

Yes, you would lose a couple of pixels for circles. I think if your figure is so cramped that you even need those (I've played the game of 6 figures x 25 subfigures each too...) then perhaps you can either slightly change the figure design, or even(!) manually set slightly larger margins :)

@brunobeltran
Copy link
Contributor Author

Yes, you would lose a couple of pixels for circles. I think if your figure is so cramped that you even need those (I've played the game of 6 figures x 25 subfigures each too...) then perhaps you can either slightly change the figure design, or even(!) manually set slightly larger margins :)

I don't disagree with you!! My advisor might though (he really likes the game of (a)-(q) subfigures). I'm still playing the game of trying to convince him that using Python is worth it, which is the impetus for trying to get something mainlined.

@brunobeltran
Copy link
Contributor Author

brunobeltran commented Mar 5, 2020

At that point the fix is more about matplotlib "feeling professional" (in the sense that it works exactly and correctly as advertised) vs it being a real problem (I can always just trim some extra whitespace in illustrator....or let the margins be a few pixels uneven....

@anntzer
Copy link
Contributor

anntzer commented Mar 6, 2020

I got your point :-) let's keep the discussion on the PR side to avoid splitting it in two places.

@brunobeltran brunobeltran linked a pull request Apr 20, 2020 that will close this issue
6 tasks
@brunobeltran brunobeltran linked a pull request Jun 22, 2020 that will close this issue
6 tasks
@github-actions
Copy link

This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!

@github-actions github-actions bot added the status: inactive Marked by the “Stale” Github Action label Jul 10, 2023
@github-actions github-actions bot added the status: closed as inactive Issues closed by the "Stale" Github Action. Please comment on any you think should still be open. label Aug 11, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 11, 2023
@anntzer
Copy link
Contributor

anntzer commented Aug 11, 2023

Let's keep this open, if open to keep track of #17198 (which seems useful in itself).

@anntzer anntzer reopened this Aug 11, 2023
@anntzer anntzer added keep Items to be ignored by the “Stale” Github Action and removed status: inactive Marked by the “Stale” Github Action status: closed as inactive Issues closed by the "Stale" Github Action. Please comment on any you think should still be open. labels Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep Items to be ignored by the “Stale” Github Action
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants