Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
ENH: inset_axes anchored-artist API#11730
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
bba682b
to5207b60
Comparejklymak commentedJul 21, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
ping@ImportanceOfBeingErnest who's concern about the API sparked this, though note the API is notexactly the same... |
5207b60
to4275e09
CompareYeah, I guess the options are discussed in#11026 (comment), so if the aim is to truely replace |
lib/matplotlib/axes/_axes.py Outdated
`.Axes.inset_axes`. | ||
A locator gets used in `Axes.set_aspect` to override the default | ||
locations... It is a function that takes an axes object and |
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 ellipsis (...
)
lib/matplotlib/axes/_axes.py Outdated
height. | ||
If a string, then locations such as "NE", "N", "NW", "W", etc, | ||
of "upper right", "top", "upper left", etc (see `~.axes.legend`) |
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.
or "upper right"
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.
And if "center" is unsopported (see above), one cannot simply link toaxes.legend
.
If a string, then locations such as "NE", "N", "NW", "W", etc, | ||
of "upper right", "top", "upper left", etc (see `~.axes.legend`) | ||
for codes. (Note we do *not* support the numerical codes). |
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.
As long as other functions likelegend
support the number codes, I would support them everywhere. Once it is decided to deprectate them, I would deprecate them all, everywhere, in one single rush. One might silently allow them here, without mentionning them specifically though. <- my personal opinion (I'm not too inclined to open that discussion again as it will just never get to an end.)
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 hear you, but someone can add that as a followup PR if they really feel strongly about it. Its a bunch of extra code for something we don't really want to support any longer.
@@ -48,10 +48,10 @@ def paintEvent(self, event): | |||
[[left, self.renderer.height - (top + height * self._dpi_ratio)], | |||
[left + width * self._dpi_ratio, self.renderer.height - top]]) | |||
reg = self.copy_from_bbox(bbox) | |||
buf = reg.to_string_argb() | |||
# buf = reg.to_string_argb() | |||
buf = memoryview(reg) |
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'm almost certain that this should not be part of this PR.
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.
Ha, no...
lib/matplotlib/tests/test_axes.py Outdated
def test_inset_codes(): | ||
"Test that the inset codes put the inset where we want..." |
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.
Single double-quotes as function docs are pretty non-standard. Not sure if it matters for tests though.
codes = ['N', 'NE', 'E', 'SE', 'S', 'SW', 'W', 'NW'] | ||
for pos, code in zip(poss, codes): | ||
axin1 = ax.inset_axes(code) | ||
np.testing.assert_allclose(axin1.get_position().get_points(), |
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 it make sense to define somebbox
es in axes coordinates here and compare to their coordinates instead? (Seems like this test is pretty susceptible to unrelated changes in the subplot positioning.)
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.
The string-code positioning has a borderpad that is in pixels, not axes co-ordinates, so knowing the bbox a-priori would just mean I was duplicating the code in the method. I guess that'd test if someone changed it ...
'center right': 'E', | ||
'lower center': 'S', | ||
'upper center': 'N', | ||
} |
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.
What about the center itself? I.e. loc code10
.
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.
Done. Thanks for all the help!
Ithink this has all the features of |
c43c4b5
tof41aa99
Comparef41aa99
tod61a940
CompareI went throughthe inset_locator_demo and indeed almost all cases are feasible with the proposed API. Here is the changed example code
While some cases become much more straight forward, e.g.
becomes
others are more intricate, e.g.
becomes:
Now there is one example, where I did not find any way to reproduce. This should create an 0.5 by 0.4 inch inset at position (0.33, 0.25) in axes coordinates.
Despite this I would still approve this, as the API is in general much simplified in contrast to the axes_grid1 solution. Still, the above shows that the axes_grid1 method has a lot of benefits and is still more generally applicable. |
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.
Despite some minor drawbacks this solution greatly simplifies the usual inset creation process. See comment above for details.
@ImportanceOfBeingErnest thanks for looking at this so carefully. There is no huge rush on this, so I will look at your cases and see if an API can be worked out that can cover those as well. Moving back to WIP... |
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
ax.inset_axes
is now merged for 3.0 (#11026) with a minimal positioning API. (ax.inset_axes([l, b, w, h])
), but folks can specify transforms that allow a pad with the edge of the axes.This PR expands the API to be similar to
ax.legend
andaxes_grid1.inset_axes
and allow a string for the position argument. So now we can doax.inset_axes('NE', width=0.4, height=0.4)
PR Checklist