-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
ENH add an inset_axes to the axes class #11026
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
Conversation
ea75792 to
0d3425a
Compare
|
In how far is it bearable or confusing for users to have
(I could imagine the problem arising because people are used to have the same function at different places, e.g. |
|
|
4c63f1c to
082b8a6
Compare
|
I am done w/ this for now, and it can be reviewed if anyone has any suggestions. (note looks like a spare debug change got into the PR, but I'll remove after/during review). |
examples/mplot3d/lines3d.py
Outdated
| y = r * np.cos(theta) | ||
|
|
||
| ax.plot(x, y, z, label='parametric curve') | ||
| l = ax.plot(x, y, z, label='parametric curve') |
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.
remove change
lib/matplotlib/axes/_axes.py
Outdated
| """ | ||
|
|
||
| def __init__(self, rect, trans, parent): | ||
| self._rect = rect |
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.
just create the transformedbbox here?
lib/matplotlib/axes/_axes.py
Outdated
|
|
||
| def __call__(self, ax, renderer): | ||
| rect = self._rect | ||
| bbox = mtransforms.Bbox.from_bounds(rect[0], rect[1], |
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.
would typically write from_bounds(*rect), not that it really matters
lib/matplotlib/axes/_axes.py
Outdated
| # This puts the rectangle into figure-relative coordinates. | ||
| bb = _inset_locator(rect, transform, self)(None, None) | ||
| # if not set, set to the same zorder as legend | ||
| zorder = kwargs.pop('zorder', 5) |
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.
kwargs.setdefault("zorder", Legend.zorder)
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.
Well, I don't think I want it tied to the legend value. And legend's value is hard coded i.e. not an rcParam....
lib/matplotlib/axes/_axes.py
Outdated
| zorder : number | ||
| Defaults to 5 (same as `.Axes.legend`). Adjust higher or lower | ||
| to change whether it is above or below data plotted on the axes. |
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.
"... on the parent axes" (or however you call it)
lib/matplotlib/axes/_axes.py
Outdated
| coordinates. | ||
| facecolor : Matplotlib color | ||
| Facecolor of the rectangle (default 'none') |
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.
full stops at end of lines (same below)
lib/matplotlib/axes/_axes.py
Outdated
| is '0.5' | ||
| alpha : number | ||
| Transparency of the rectangle and connector lines. Default 0.5 |
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.
default is
lib/matplotlib/axes/_axes.py
Outdated
| ------- | ||
| rectangle_patch, connector_lines : | ||
| `.Patches.Rectagle`, (four-tupple `.Patches.ConnectionPatch`) one |
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.
Rectangle tuple
lib/matplotlib/axes/_axes.py
Outdated
| # decide which two of the lines to keep visible.... | ||
| pos = inset_ax.get_position() | ||
| bboxins = pos.transformed(self.figure.transFigure) | ||
| rectbbox = mtransforms.Bbox.from_bounds(rect[0], rect[1], |
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.
from_bounds(*rect)
lib/matplotlib/axes/_axes.py
Outdated
| ------- | ||
| rectangle_patch, connector_lines : | ||
| `.Patches.Rectagle`, (four-tupple `.Patches.ConnectionPatch`) one |
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.
rectangle, tuple
| self.tables = [] | ||
| self.artists = [] | ||
| self.images = [] | ||
| self.child_axes = [] |
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.
stash them into self.artists? I'd rather work towards unifying all these containers rather than adding more of them...
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.
Seec omment below for why not implimented....
|
What I definitely like better about the would be which is a bit cumbersome to write for such standard case. |
|
If that's a desirable feature (not certain yet, but possibly), certainly it should be pulled to core so that legend, text annotations and whatnot can also benefit from it? |
|
@anntzer I think in its current form So for the core matplotlib, one should probably spend some more thoughts about how to best design such function to be powerful, but not as confusing. |
|
@anntzer and @ImportanceOfBeingErnest, thanks for the feedback. I think the logic for padded boxes is indeed in
What might make sense is to add a "pad" parameter in appropriate units (probably points, versus pixels), that would offset the new inset axes from the rectangle provided. Then if you want upper right you do: w = 0.2, h=0.2
axins = ax.inset_axes([1-w, 1-h, w, h], pad=5)I don't think thats hard to do, though I'll have to check how its done to be robust under figure size changes. Could have an Not sure how all that would work with set aspect ratios for the child axes. Would something like that satisfy the need? Conversely, I'm not 100% against just moving the EDIT: cross-post! |
|
To be clear, I think this should go in without padding support, because it's basically using the same positioning syntax as |
|
I guess personally I don't have a problem with padding just being a fraction of the axes size (i.e. I can hard code it into |
|
I'm thinking more in terms of ease of use. One can always argue that if people want to have full control they can reach their goal via the axes_grid1 inset or custom transforms etc. In that sense, something like which can be called just as |
|
I think "loc=number" is a terrible API. At least that should be using strings (which are also allowed with legend, but I would want new APIs to only use strings). |
81f5aed to
435db99
Compare
435db99 to
b712c7b
Compare
|
Rebased and squashed. Be nice to get this in relatively soon because then I can move onto axes for secondary scales... |
|
recycled for CI |
b712c7b to
edd3a13
Compare
|
rebased |
edd3a13 to
37aefb7
Compare
|
This just needs one more review to go in as a pretty useful 3.0 feature. OTOH, understand if it needs to get pushed 6 months... |
lib/matplotlib/axes/_axes.py
Outdated
| _parent = parent | ||
|
|
||
| def inset_locator(ax, renderer): | ||
| bbox = _rect |
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.
Is this line actually doing anything other than renaming a variable? I think it is superfluous.
37aefb7 to
0abd014
Compare
efiring
left a comment
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.
I would like to see this go in ASAP, preferably with one of the variations on naming that was discussed: drop the "from...", so it is "inset_axes()" and "indicate_inset()", and use "bounds" consistently for the corresponding variable name in private as well as public functions.
82b88b7 to
35c29c2
Compare
35c29c2 to
0d604bf
Compare
0d604bf to
a5b27e9
Compare
PR Summary
and we are back to
inset_axes....@efiring suggested just going back to
inset_axes. Since that was what we started with, I didn't object. The original argument for the longer name was if the API was to include aloc='SW'API, instead of theboundsAPI I've used here. Thats still possible by type-checking theboundsargument.EDIT:
Name bikeshedding:
Proposal after comments below:
rectwas too ambiguous,lbwhtoo obscure,boundsis already used for bboxes (as is extents for lbrt)...Comments on names still welcome...
Old PR descr
Adds
ax.inset_axes_from_rect. Pretty straight forward, but note that thetransDataone moves under zoom and pan.Note that I chose not to go with the
bbox_to_anchor, andlocapproach. I understand that stuff, but it seems needlessly complicated. Maybe as a refinement.Getting closer: This is a duplication of https://matplotlib.org/gallery/axes_grid1/inset_locator_demo2.html#sphx-glr-gallery-axes-grid1-inset-locator-demo2-py
EDIT: methods renamed
inset_axes_rectto allow other APIs in the future.PR Checklist