-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
@@ -23,7 +23,7 @@ | |||
x = r * np.sin(theta) | |||
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
lib/matplotlib/axes/_base.py
Outdated
@@ -1048,6 +1048,7 @@ def cla(self): | |||
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
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 thebounds
API I've used here. Thats still possible by type-checking thebounds
argument.EDIT:
Name bikeshedding:
Proposal after comments below:
rect
was too ambiguous,lbwh
too obscure,bounds
is 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 thetransData
one moves under zoom and pan.Note that I chose not to go with the
bbox_to_anchor
, andloc
approach. 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_rect
to allow other APIs in the future.PR Checklist