#19768 (comment) suggested that we should have some kind of container for the output ofindicate_inset[_zoom].#23424 (comment) suggested that it should be drawn as a single path to avoid double-alpha where the box and connectors overlap. This PR is my attempt to implement that.

The new artist has convenience methods to update colour/linestyle/etc for the whole thing, but the user can also get hold of individual patches and update styles on those. The current doctrings ofindicate_inset[_zoom] specifically mention updating the visibility of the connectors, but a user might also reasonably want a different colour or linestyle for the connectors than for the box.

The logic fromindicate_inset_zoom that decided where the box should be is now methodInsetIndicator._bounds_from_inset_ax so we can reuse it. Whenever theconnectors attribute is accessed (including at draw) the box bounds and position of connectors (if any) get updated using the current inset axes x- and y-limits. This thereforecloses#19768 and we can do things like

importmatplotlib.pyplotaspltimportmatplotlib.animationasanimationimportnumpyasnpx=np.arange(100)y=x+np.sin(x)*5fig,ax=plt.subplots()ax.plot(x,y)ax_ins=ax.inset_axes([0.6,0.1,0.3,0.3])ax_ins.plot(x,y)ax_ins.set_xlim([0,10])ax_ins.set_ylim([0,10])ind=ax.indicate_inset_zoom(ax_ins)forconninind.connectors:# In this example all the connectors are invisible by default - I'm unclear why that is.conn.set_visible(True)defanimate(i):ax_ins.set_xlim([i,i+10])ax_ins.set_ylim([i-5,i+15])returnax_ins,ani=animation.FuncAnimation(fig,animate,interval=100,frames=90)plt.show()

Styles for the connectors are now inherited from the parent artist when the connectors are created. This thereforecloses#23424. Within thedraw method, we check which connectors have the same style properties as the rectangle (currently four specific properties - maybe there is a better way). Those that do are drawn with the rectangle as a compound path.

This also inadvertentlyfixes#23425 since I haven't implementedget_window_extent orget_tight_bbox, so constrained layout will get a null tight bbox. I would like advice on whether I should implement one or both of these and what it should include. For the purposes of layout engines is does not seem useful to include the connectors, but maybe there are other uses for these methods?

PR checklist

"closes #0000" is in the body of the PR description tolink the related issue
new and changed code istested
Plotting related features are demonstrated in anexample
New Features andAPI Changes are noted with adirective and release note
Documentation complies withgeneral anddocstring guidelines

rcomer added the API: changes label

Mar 30, 2024

rcomer added this to thev3.10.0 milestone

Mar 30, 2024

Copy link

MemberAuthor

rcomer commentedMar 30, 2024

Added a__getitem__ method in an effort to provide a deprecation pathway. Althoughindicate_inset[_zoom] are still marked as experimental, I suspect the patternbox, connectors = ax.indicate_inset... is pretty common.

rcomer changed the title~~POC: createIndicateInset patch~~POC: createInsetIndicator patch

Apr 22, 2024

rcomer force-pushed theindicate-inset-patch branch 2 times, most recently fromfce57eb to3d725eaCompare

May 12, 2024 19:18

Copy link

MemberAuthor

rcomer commentedJul 27, 2024

Perhaps instead of inheriting fromRectangle, this should be a container artist with both the rectangle and the connectors as children. Then users who want more control could get hold of the child artists and update colours, etc. on them individually 🤔

Copy link

Member

story645 commentedJul 28, 2024•
edited
Loading

Then users who want more control could get hold of the child artists and update colours, etc. on them individually

That sounds like a fairly common use case to me so I think that'd be awesome. Is there an advantage to the rectangle approach/downside to making it a container?

Copy link

MemberAuthor

rcomer commentedJul 28, 2024

Is there an advantage to the rectangle approach/downside to making it a container?

Not that I can think of. I think I just had in mind from#23424 (comment) that the new object should be aPatch and inheriting fromRectangle seemed like the obvious way to do that. Having said that, I'm not sure what defines aPatch and maybe that fact that the connectors are already children that you can configure separately from the whole thing means it doesn't really fit the definition.

Copy link

Member

story645 commentedJul 28, 2024•
edited
Loading

I'm not sure what defines a Patch

Technicallyartist w/ face color + edgecolor & for the most part I've always thought of it as roughly the MPL abstraction of an area mark b/c that's basically the space of visual encodings we allow/can apply to most of our patch artists:

Where size and shape are controlled by the drawing of the shape, while for example the choice of marker determines shape and marker size controls the size.

So for what it's worth, I agree w/ your intuition that a container Artist is probably a better abstraction for what's actually going on here.

rcomer force-pushed theindicate-inset-patch branch from3d725ea to745c737Compare

August 11, 2024 18:59

github-actionsbot added the Documentation: APIfiles in lib/ and doc/api label

Aug 11, 2024

rcomer changed the title~~POC: createInsetIndicator patch~~POC: createInsetIndicator artist

Aug 11, 2024

rcomer force-pushed theindicate-inset-patch branch from745c737 tob8d7469Compare

August 11, 2024 19:15

Copy link

MemberAuthor

rcomer commentedAug 11, 2024•
edited
Loading

OK, theInsetIndicator is now a container artist instead of a patch. I couldn't see which module it would obviously fit in, so made a new one.

One side-effect of this is that we no longer get the axes limits updated around the artist, which is done inadd_patch here:

matplotlib/lib/matplotlib/axes/_base.py

Line 2414 in1e98377

self._update_patch_limits(p)

So my test image changed from

I guess it would be pretty easy to re-instate that but I'm not sure if it matters. Most of the time there will be a bunch of other artists that this one isindicating.

Still to do:

~~Add changenotes~~
Decide what, if anything, to do aboutget_window_extent/get_tightbbox

rcomer force-pushed theindicate-inset-patch branch fromb8d7469 tocbe1aa1Compare

August 12, 2024 10:56

github-actionsbot removed the Documentation: APIfiles in lib/ and doc/api label

Aug 12, 2024

Copy link

MemberAuthor

rcomer commentedAug 12, 2024

Added changenotes and directives, though I'm not sure if the behaviour change documentation forindicate_inset[_zoom] is really needed since these methods were marked as experimental...?

rcomer marked this pull request as ready for review

August 12, 2024 10:57

rcomer force-pushed theindicate-inset-patch branch fromcbe1aa1 to9deb35bCompare

August 12, 2024 12:07

This comment was marked as resolved.

Copy link

MemberAuthor

rcomer commentedAug 14, 2024

I hid the alpha discussion because that was addressed by#28710 and is not needed for the reviewers of this PR.

story645 self-assigned this

Aug 15, 2024

Copy link

Member

tacaswell commentedAug 15, 2024

Decide what, if anything, to do about get_window_extent/get_tightbbox

It is also used byfig.savefig(..., bbox_inches='tight') so if we do not implement it then there is a high risk that people using the inline backend may be able to drive them selves to a state where we clip something we should not have, however as both the host and inset axes do implement those methods, I'm struggling to come up with an arangement of the two axes that would result in the connector getting clipped...

When you said "container" I was worried you were following what we do with e.g. error bar where we return atuple subclass, but I think what you implemented is the right way to go.

Copy link

Member

story645 commentedAug 15, 2024

why did the lower left and upper right segments disappear?

Copy link

Member

jklymak commentedAug 16, 2024

I don’t know about dragging but if you pan/zoom it can end up outside the axes.

yeah, that is all a bit tricky. The rectangle would ideally clip to the parent axes; probably easy. The connectors would ideally clip the parent axes as well, but only on the side that links to the rectangle. I'm not aware that we have a way to do that and I'm not sure it's worth the hassle to make it. Either you are doing this for data exploration, in which case a little mess is OK, or you are doing it for a final plot, in which case you can clean up the positions then.

Copy link

MemberAuthor

rcomer commentedAug 16, 2024

why did the lower left and upper right segments disappear?

You mean the connectors? Only 2 of those are visible by default.
https://matplotlib.org/stable/gallery/subplots_axes_and_figures/zoom_inset_axes.html

Copy link

Member

story645 commentedAug 16, 2024

why did the lower left and upper right segments disappear?
You mean the connectors? Only 2 of those are visible by default.
https://matplotlib.org/stable/gallery/subplots_axes_and_figures/zoom_inset_axes.html

Thanks! Sorry for not checking the docs 🤦‍♀️
Am clearly confused by that API choice but is out of scope here.

rcomer force-pushed theindicate-inset-patch branch from9deb35b to9134a5cCompare

August 16, 2024 19:31

story645 removed their assignment

Aug 16, 2024

QuLogic reviewed

Aug 17, 2024

View reviewed changes

lib/matplotlib/axes/_axes.py OutdatedShow resolvedHide resolved

lib/matplotlib/inset.pyShow resolvedHide resolved

lib/matplotlib/inset.py OutdatedShow resolvedHide resolved

lib/matplotlib/inset.py

		for conn in self.connectors or []:
		if conn.get_visible():
		drawn = False
		for s in _shared_properties:

Copy link

Member

QuLogicAug 17, 2024

Choose a reason for hiding this comment

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

Isn't it also the case that if the non-shared properties differ, then you can't merge the paths? For example,zorder isn't listed as shared, but might make a difference.

Copy link

MemberAuthor

rcomerAug 17, 2024

Choose a reason for hiding this comment

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

Yeah, I wasn't super-happy about doing this with a hard-coded list, but so far haven't thought of a better way. Very happy to take suggestions/pointers.

... oh I just realised I could useArtistInspector.get_setters to make a complete list - maybe that's part of the solution, though we'd have to actively skip comparing some properties liketransform 🤔

Copy link

MemberAuthor

rcomerAug 17, 2024

Choose a reason for hiding this comment

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

I also just realised that I currently have thezorder going toRectangle in the__init__ when it should be getting applied toInsetIndicator. So I need a better way to keep the properties straight at that end too.

lib/matplotlib/tests/test_inset.py OutdatedShow resolvedHide resolved

rcomer force-pushed theindicate-inset-patch branch 4 times, most recently from6e106e9 to143a228Compare

August 18, 2024 13:16

story645 mentioned this pull request

Aug 22, 2024

[MNT]: document linestyle as an mpltype#28745

Open

story645 reviewed

Aug 22, 2024

View reviewed changes

doc/users/next_whats_new/inset_indicator.rst OutdatedShow resolvedHide resolved

lib/matplotlib/axes/_axes.py

Comment on lines +476 to +485

		inset_indicator.rectangle : `.Rectangle`
		The indicator frame.

		inset_indicator.connectors : 4-tuple of `.patches.ConnectionPatch`
		The four connector lines connecting to (lower_left, upper_left,
		lower_right upper_right) corners of inset_ax. Two lines are
		set with visibility to False, but the user can set the
		visibility to True if the automatic choice is not deemed correct.

Copy link

Member

story645Aug 22, 2024•
edited
Loading

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 make this an interpolated doc, even if it's only used in like two places? That way it can be defined in the module? Only thinking about this b/c of how it's being done for colorizer:

matplotlib/lib/matplotlib/colorizer.py

Lines 29 to 34 incf9a943

	mpl._docstring.interpd.update(
	colorizer_doc="""\
	colorizer : `~matplotlib.colorizer.Colorizer` or None, default: None
	The Colorizer object used to map color to data. If None, a Colorizer
	object is created base on norm and cmap.""",
	)

Copy link

Member

story645Aug 22, 2024

Choose a reason for hiding this comment

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

Possibly can do something even more direct but I do not follow this example at allhttps://matplotlib.org/devdocs/devel/document.html#keyword-arguments

Copy link

MemberAuthor

rcomerAug 25, 2024

Choose a reason for hiding this comment

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

Do we have any general guidance about when it makes sense to do this rather than copy/paste? As you say it's only in two places and I am keenly aware that I have a lot more copy/paste going on in the artist'sset_* docstrings.