| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Is this meant to be used by tight_layout, such that one could call fig.tight_layout(bbox_extra_artists=[])? |
Sorry, something went wrong.
|
I think thats possible, though this PR doesn't do that. Not quite sure what happens if you specify an artist that isn't in the axes (tight layout would have to accept all the artists for each axes). |
Sorry, something went wrong.
|
See update above: rebased and added artist.inbbox to determine if an artist is in a bbox calculation or not. |
Sorry, something went wrong.
|
I am happy with this in principle, but we probably need to add the get_* and set_* methods to keep the API guessable. We need a better name than inbbox as bbox is a bit arcane and inbbox reads to be as a question not an instruction. |
Sorry, something went wrong.
|
|
||
| def get_tightbbox(self, renderer, call_axes_locator=True): | ||
| def get_tightbbox(self, renderer, call_axes_locator=True, | ||
| bbox_extra_artists=None): |
There was a problem hiding this comment.
please do not add this extra kwarg.
Sorry, something went wrong.
There was a problem hiding this comment.
Sorry, I screwed up the PR during a rebase... The reason for this is so that the artist logic that was in backend_bases.print_figure could be refactored out and reused. So this needs to be here to make that
function work. Could make a private kwarg (is there such a thing?)
Sorry, something went wrong.
|
added setters and getters, and changed name to artist.set/get_in_layout and made artist._in_layout private. |
Sorry, something went wrong.
|
@efiring Thanks for the review. Outstanding questions for others:
bbox : `.BboxBase`
containing the bounding box (in figure pixel co-ordinates).
I don't think this was ever properly specified before this PR. Probably because get_tightbbox and get_window_extents verge on private methods. |
Sorry, something went wrong.
|
re 1. From the pythonic point of view this would just be an attribute artist.in_layout. A property does not make sense here. Properties are basically there to attach code to reading or writing an attribute without changing the signature. You can just use the attribute and turn it into a property later if needed. The only argument for set/get would be that it's widely used within matplotlib. However there are also a few counter-examples artist.axes, artist.stale. It's up to one of the more senior devs to decide which way to go. Personally, I'd have a slight preference for the attribute solution. |
Sorry, something went wrong.
Sorry, meant "attribute". OTOH I think the issue is documentation - how does the user discover this attribute in the docs? |
Sorry, something went wrong.
|
An attribute can have a docstring, and sphinx can include the documentation either by listing the attribute along with other explicit members, or via the autoattribute directive: http://www.sphinx-doc.org/en/stable/ext/autodoc.html I think @tacaswell really doesn't want to start doing this (using bare attributes) until the massive traitification is done, however. But maybe he will change his mind for cases like this. |
Sorry, something went wrong.
|
I’d be all for either form, with slight preference for the attribute. @tacaswell? |
Sorry, something went wrong.
|
I accept @tacaswell's argument that keeping the machinery provided by the setters and getters is worthwhile for this, so ignore my suggestion to reduce it to a bare attribute. |
Sorry, something went wrong.
There was a problem hiding this comment.
conditional on CI
Sorry, something went wrong.
|
self merging as reviewed and passed CI |
Sorry, something went wrong.
|
Grrrrr. This has a funny interaction w/ zooming. If I zoom, then the clip path of an artist is not set properly until after a draw. But all the automatic layout depends on being able to get a sensible tighbbox during draw... Darn race conditions.... |
Sorry, something went wrong.
|
I just spent several hours tracking down why my annotations were getting placed incorrectly after updating from Matplotlib v2.2.3 to Matplotlib v3.3.1. The problem turned out to be ax.get_tightbbox() was not returning the same result as it did under Matplotlib v2.2.3. I am, however, pleased that I can get the old behavior with ax.get_tightbbox(bbox_extra_artists=[]). Thanks for keeping some degree of backwards compatibility. |
Sorry, something went wrong.
|
You can also twiddle manually: artist.set_in_layout(False)... |
Sorry, something went wrong.
…udes tick marks and tick mark labels, but not the axes lables, axes title, or other annotations. As of matplotlib 3.0, however, mpl_ax.get_tightbbox() returns the bounding box that includes the axes labels, axes title, and other annotations (see matplotlib/matplotlib#10682). This new behavior was messing up my axis label placement. Fortunately, one can still get the original behavior via mpl_ax.get_tightbbox(bbox_extra_artists = []), so the axes.tight_bbox property was updated to use this new kwarg.
| Back | FazBrowse Home | New Git URL |
PR Summary
This PR takes the code that was in backend_bases.py in savefig for determining a figure bounding box, and puts it in ax.get_tightbbox and fig.get_tightbbox, via a refactoring into Artists.get_tightbbox.
Supercedes #10678 and extends already merged #9164
In previous PRs I was piecemeal adding new types of artists, but noted that fig.savefig(bbox_inches='tight') was actually doing the correct thing and putting the bbox around all the artists.
To get the old behaviour back, a new kwarg has been added:
ax.get_tightbbox(bbox_extra_artists=[])
So now the following code
returns:
This is a breaking change, in that anyone who was expecting the bbox to not include artists, but just the axes axis elements and labels, will now get a different behaviour. They can get the old behaviour as above (specifying the empty bbox_extra_artists kwarg. OTOH, willing be told this is too big an API change. (it of course still needs an API change note, and likely a test or two somewhere).
UPDATE 28 April:
This adds the artist property artist.set/get_in_layout (as discussed w/ @efiring et al on the weekly call) which specifies if an artist should be included in the tight_bbox calculation. This allows toggling artists in the tight layout or constrained layout calculations.
So, now we can also do:
and get
Note that this also works for legends, which often get put outside axes and hence may not want to be part of the tight_layout machinery.
Discussion: Should artists that did not get included in the old tight_layout be set to False by default? I'm somewhat against this because it means users have to guess or query what state the attribute is in...
PR Checklist