Skip to content

Inset axes bug and docs fix #14278

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 6, 2019
Merged

Inset axes bug and docs fix #14278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e46968
TST: Axes.indicate_inset failure
clbarnes May 20, 2019
86f5fb0
FIX: Axes.indicate_inset* methods
clbarnes May 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions doc/api/next_api_changes/2019-05-21-CB.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
``matplotlib.axes.Axes.indicate_inset`` returns a 4-tuple as documented
-----------------------------------------------------------------------

In <= 3.1.0, ``matplotlib.axes.Axes.indicate_inset`` and
``matplotlib.axes.Axes.indicate_inset_zoom`` were documented as returning
a 4-tuple of ``matplotlib.patches.ConnectionPatch`` es, where in fact they
returned a 4-length list.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I'm not seeing the difference here. What is the advantage of the change below? If its just because we don't think a list is a tuple, then I'd argue just changing the docs to say it is a list. But practically, what is the difference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand. Lists and tuples are different. They do different things. They have different methods. At the very least, the documentation should match the implementation.

If you tell someone you're giving them a duck and then show up with a cat, they're going to be annoyed that they've spent some time digging a pond.

I'd argue just changing the docs to say it is a list

This is what I did initially. However, in this case, the return value has a fixed length and the indices are meaningful, so an optional tuple is closer to the meaning of what's being represented. I suggested the change in an earlier comment and said my only reservation was about it being a breaking API change, and you responded that breaking changes were fair game as it's an experimental feature.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I was looking at the wrong change. In general it’s better not to have too many style changes unless they help pass PEP8 with fewer exceptions.

I’m not clear if a tuple or list is particularly better here. I guess the fact that it’s always a four tuple would argue for that. My apologies that my previous comment was taken to mean I thought it should be changed, just that it could if there is a good reason.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning a tuple makes sense semantically.

They now correctly return a 4-tuple.
``matplotlib.axes.Axes.indicate_inset`` would previously raise an error if
the optional *inset_ax* was not supplied; it now completes successfully,
and returns *None* instead of the tuple of ``ConnectionPatch`` es.

49 changes: 31 additions & 18 deletions lib/matplotlib/axes/_axes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,20 @@
import numpy as np
from numpy import ma

from matplotlib import _preprocess_data, rcParams
import matplotlib.category as _ # <-registers a category unit converter
import matplotlib.cbook as cbook
import matplotlib.collections as mcoll
import matplotlib.colors as mcolors
import matplotlib.contour as mcontour
import matplotlib.category as _ # <-registers a category unit converter
import matplotlib.dates as _ # <-registers a date unit converter
import matplotlib.docstring as docstring
import matplotlib.image as mimage
import matplotlib.legend as mlegend
import matplotlib.lines as mlines
import matplotlib.markers as mmarkers
import matplotlib.mlab as mlab
import matplotlib.path as mpath
import matplotlib.patches as mpatches
import matplotlib.path as mpath
import matplotlib.quiver as mquiver
import matplotlib.stackplot as mstack
import matplotlib.streamplot as mstream
Expand All @@ -31,9 +30,10 @@
import matplotlib.ticker as mticker
import matplotlib.transforms as mtransforms
import matplotlib.tri as mtri
from matplotlib.container import BarContainer, ErrorbarContainer, StemContainer
from matplotlib import _preprocess_data, rcParams
from matplotlib.axes._base import _AxesBase, _process_plot_format
from matplotlib.axes._secondary_axes import SecondaryAxis
from matplotlib.container import BarContainer, ErrorbarContainer, StemContainer

try:
from numpy.lib.histograms import histogram_bin_edges
Expand Down Expand Up @@ -514,9 +514,12 @@ def indicate_inset(self, bounds, inset_ax=None, *, transform=None,
rectangle_patch : `.Patches.Rectangle`
Rectangle artist.

connector_lines : 4-tuple of `.Patches.ConnectionPatch`
One for each of four connector lines. Two are set with visibility
to *False*, but the user can set the visibility to True if the
connector_lines : optional 4-tuple of `.Patches.ConnectionPatch`
Each of four connector lines coming from the given rectangle
on this axes in the order lower left, upper left, lower right,
upper right: *None* if *inset_ax* is *None*.
Two are set with visibility to *False*,
but the user can set the visibility to *True* if the
automatic choice is not deemed correct.

"""
Expand All @@ -535,25 +538,31 @@ def indicate_inset(self, bounds, inset_ax=None, *, transform=None,
zorder=zorder, label=label, transform=transform, **kwargs)
self.add_patch(rectpatch)

connects = []

if inset_ax is not None:
# want to connect the indicator to the rect....
connects = []
xr = [bounds[0], bounds[0]+bounds[2]]
yr = [bounds[1], bounds[1]+bounds[3]]
for xc in range(2):
for yc in range(2):
xyA = (xc, yc)
xyB = (xr[xc], yr[yc])
connects += [mpatches.ConnectionPatch(xyA, xyB,
connects.append(
mpatches.ConnectionPatch(
xyA, xyB,
'axes fraction', 'data',
axesA=inset_ax, axesB=self, arrowstyle="-",
zorder=zorder, edgecolor=edgecolor, alpha=alpha)]
zorder=zorder, edgecolor=edgecolor, alpha=alpha
)
)
self.add_patch(connects[-1])
# 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(
*bounds).transformed(transform)
*bounds
).transformed(transform)
x0 = rectbbox.x0 < bboxins.x0
x1 = rectbbox.x1 < bboxins.x1
y0 = rectbbox.y0 < bboxins.y0
Expand All @@ -563,7 +572,7 @@ def indicate_inset(self, bounds, inset_ax=None, *, transform=None,
connects[2].set_visible(x1 == y0)
connects[3].set_visible(x1 ^ y1)

return rectpatch, connects
return rectpatch, tuple(connects) if connects else None

def indicate_inset_zoom(self, inset_ax, **kwargs):
"""
Expand All @@ -583,25 +592,29 @@ def indicate_inset_zoom(self, inset_ax, **kwargs):
chosen so as to not overlap with the indicator box.

**kwargs
Other *kwargs* are passed on to `.Axes.inset_rectangle`
Other *kwargs* are passed on to `.Axes.indicate_inset`

Returns
-------
rectangle_patch : `.Patches.Rectangle`
Rectangle artist.

connector_lines : 4-tuple of `.Patches.ConnectionPatch`
One for each of four connector lines. Two are set with visibility
to *False*, but the user can set the visibility to True if the
automatic choice is not deemed correct.
Each of four connector lines coming from the rectangle drawn on
this axis, in the order lower left, upper left, lower right,
upper right.
Two are set with visibility to *False*, but the user can
set the visibility to *True* if the automatic choice is not deemed
correct.

"""

xlim = inset_ax.get_xlim()
ylim = inset_ax.get_ylim()
rect = [xlim[0], ylim[0], xlim[1] - xlim[0], ylim[1] - ylim[0]]
rect = (xlim[0], ylim[0], xlim[1] - xlim[0], ylim[1] - ylim[0])
rectpatch, connects = self.indicate_inset(
rect, inset_ax, **kwargs)
rect, inset_ax, **kwargs
)

return rectpatch, connects

Expand Down
30 changes: 30 additions & 0 deletions lib/matplotlib/tests/test_axes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5959,6 +5959,35 @@ def test_tick_padding_tightbbox():
assert bb.y0 < bb2.y0


def test_inset():
"""
Ensure that inset_ax argument is indeed optional
"""
dx, dy = 0.05, 0.05
# generate 2 2d grids for the x & y bounds
y, x = np.mgrid[slice(1, 5 + dy, dy),
slice(1, 5 + dx, dx)]
z = np.sin(x) ** 10 + np.cos(10 + y * x) * np.cos(x)

fig, ax = plt.subplots()
ax.pcolormesh(x, y, z)
ax.set_aspect(1.)
ax.apply_aspect()
# we need to apply_aspect to make the drawing below work.

xlim = [1.5, 2.15]
ylim = [2, 2.5]

rect = [xlim[0], ylim[0], xlim[1] - xlim[0], ylim[1] - ylim[0]]

rec, connectors = ax.indicate_inset(bounds=rect)
assert connectors is None
fig.canvas.draw()
xx = np.array([[1.5, 2.],
[2.15, 2.5]])
assert np.all(rec.get_bbox().get_points() == xx)


def test_zoom_inset():
dx, dy = 0.05, 0.05
# generate 2 2d grids for the x & y bounds
Expand All @@ -5981,6 +6010,7 @@ def test_zoom_inset():
axin1.set_aspect(ax.get_aspect())

rec, connectors = ax.indicate_inset_zoom(axin1)
assert len(connectors) == 4
fig.canvas.draw()
xx = np.array([[1.5, 2.],
[2.15, 2.5]])
Expand Down