Create `InsetIndicator` artist #27996

rcomer · Mar 30, 2024

PR summary

#19768 (comment) suggested that we should have some kind of container for the output of indicate_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 of indicate_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 from indicate_inset_zoom that decided where the box should be is now method InsetIndicator._bounds_from_inset_ax so we can reuse it. Whenever the connectors 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 therefore closes #19768 and we can do things like

import matplotlib.pyplot as plt
import matplotlib.animation as animation
import numpy as np

x = np.arange(100)
y = x + np.sin(x) * 5

fig, 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)
for conn in ind.connectors:
    # In this example all the connectors are invisible by default - I'm unclear why that is.
    conn.set_visible(True)

def animate(i):
    ax_ins.set_xlim([i, i+10])
    ax_ins.set_ylim([i-5, i+15])
    return ax_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 therefore closes #23424. Within the draw 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 inadvertently fixes #23425 since I haven't implemented get_window_extent or get_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 to link the related issue
new and changed code is tested
Plotting related features are demonstrated in an example
New Features and API Changes are noted with a directive and release note
Documentation complies with general and docstring guidelines

rcomer · Mar 30, 2024

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

rcomer · Jul 27, 2024

Perhaps instead of inheriting from Rectangle, 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 🤔

story645 · Jul 28, 2024

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?

rcomer · Jul 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 a Patch and inheriting from Rectangle seemed like the obvious way to do that. Having said that, I'm not sure what defines a Patch 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.

story645 · Jul 28, 2024

I'm not sure what defines a Patch

Technically artist 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 · Aug 11, 2024

OK, the InsetIndicator 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 in add_patch here:

matplotlib/lib/matplotlib/axes/_base.py

Line 2414 in 1e98377

self._update_patch_limits(p)

So my test image changed from

to

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 is indicating.

Still to do:

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

rcomer · Aug 12, 2024

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

rcomer · Aug 14, 2024

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

tacaswell · Aug 15, 2024

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

It is also used by fig.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 a tuple subclass, but I think what you implemented is the right way to go.

story645 · Aug 15, 2024

why did the lower left and upper right segments disappear?

jklymak · Aug 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.

rcomer · Aug 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

story645 · Aug 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.

lib/matplotlib/axes/_axes.py

lib/matplotlib/inset.py

QuLogic · Aug 17, 2024

lib/matplotlib/inset.py

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


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.

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 use ArtistInspector.get_setters to make a complete list - maybe that's part of the solution, though we'd have to actively skip comparing some properties like transform 🤔

I also just realised that I currently have the zorder going to Rectangle in the __init__ when it should be getting applied to InsetIndicator. So I need a better way to keep the properties straight at that end too.

lib/matplotlib/tests/test_inset.py

doc/users/next_whats_new/inset_indicator.rst

story645 · Aug 22, 2024

lib/matplotlib/axes/_axes.py

+            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.
+


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 in cf9a943

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*.""",

)

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

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's set_* docstrings.

Trying to figure that out w/ #28746 and probably a @timhoffm question?

lib/matplotlib/inset.py

story645 · Aug 22, 2024

lib/matplotlib/tests/test_inset.py

+        if conn.get_visible():
+            # Make one visible connector a different style
+            conn.set_linestyle('dashed')
+            conn.set_color('blue')
+            break


shouldn't you know which connector this is b/c this is deterministic? basically why loop?

I was just too lazy to figure out which index it would be...

lib/matplotlib/inset.py

QuLogic · Sep 12, 2024

Is this no longer PoC (as in title)?

rcomer · Sep 17, 2024

I just noticed I forgot to update the deprecation warning when the patch became an artist. Now fixed.

rcomer added the API: changes label Mar 30, 2024

rcomer added this to the v3.10.0 milestone Mar 30, 2024

rcomer changed the title ~~POC: create IndicateInset patch~~ POC: create InsetIndicator patch Apr 22, 2024

rcomer force-pushed the indicate-inset-patch branch 2 times, most recently from fce57eb to 3d725ea Compare May 12, 2024 19:18

rcomer force-pushed the indicate-inset-patch branch from 3d725ea to 745c737 Compare August 11, 2024 18:59

github-actions bot added the Documentation: API files in lib/ and doc/api label Aug 11, 2024

rcomer changed the title ~~POC: create InsetIndicator patch~~ POC: create InsetIndicator artist Aug 11, 2024

rcomer force-pushed the indicate-inset-patch branch from 745c737 to b8d7469 Compare August 11, 2024 19:15

rcomer force-pushed the indicate-inset-patch branch from b8d7469 to cbe1aa1 Compare August 12, 2024 10:56

github-actions bot removed the Documentation: API files in lib/ and doc/api label Aug 12, 2024

rcomer marked this pull request as ready for review August 12, 2024 10:57

rcomer force-pushed the indicate-inset-patch branch from cbe1aa1 to 9deb35b Compare August 12, 2024 12:07

This comment was marked as resolved.

Sign in to view

story645 self-assigned this Aug 15, 2024

rcomer force-pushed the indicate-inset-patch branch from 9deb35b to 9134a5c Compare August 16, 2024 19:31

story645 removed their assignment Aug 16, 2024

QuLogic reviewed Aug 17, 2024

View reviewed changes

rcomer force-pushed the indicate-inset-patch branch 4 times, most recently from 6e106e9 to 143a228 Compare 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

QuLogic reviewed Aug 24, 2024

View reviewed changes

lib/matplotlib/inset.py Outdated Show resolved Hide resolved

rcomer force-pushed the indicate-inset-patch branch from e5ad7fc to d2cd137 Compare August 25, 2024 19:03

github-actions bot added the status: needs rebase label Sep 7, 2024

rcomer changed the title ~~POC: create InsetIndicator artist~~ Create InsetIndicator artist Sep 12, 2024

rcomer force-pushed the indicate-inset-patch branch from d2cd137 to adb8a95 Compare September 12, 2024 20:30

github-actions bot removed the status: needs rebase label Sep 12, 2024

QuLogic approved these changes Sep 14, 2024

View reviewed changes

github-actions bot added the status: needs rebase label Sep 17, 2024

rcomer force-pushed the indicate-inset-patch branch from adb8a95 to 4d944bb Compare September 17, 2024 17:56

github-actions bot removed the status: needs rebase label Sep 17, 2024

Add InsetIndicator artist

b833055

rcomer force-pushed the indicate-inset-patch branch from 4d944bb to b833055 Compare September 17, 2024 18:20

tacaswell merged commit ea78e25 into matplotlib:main Sep 18, 2024
43 checks passed

rcomer deleted the indicate-inset-patch branch September 18, 2024 19:43

rcomer mentioned this pull request Sep 18, 2024

Forward select Rectangle kwargs to Connection patch #24082

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create `InsetIndicator` artist #27996

Create `InsetIndicator` artist #27996

rcomer commented Mar 30, 2024 •

edited

Loading

rcomer commented Mar 30, 2024

rcomer commented Jul 27, 2024

story645 commented Jul 28, 2024 •

edited

Loading

rcomer commented Jul 28, 2024

story645 commented Jul 28, 2024 •

edited

Loading

rcomer commented Aug 11, 2024 •

edited

Loading

rcomer commented Aug 12, 2024

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

rcomer commented Aug 14, 2024

tacaswell commented Aug 15, 2024

story645 commented Aug 15, 2024

jklymak commented Aug 16, 2024

rcomer commented Aug 16, 2024

story645 commented Aug 16, 2024

QuLogic Aug 17, 2024

rcomer Aug 17, 2024

rcomer Aug 17, 2024

story645 Aug 22, 2024 •

edited

Loading

story645 Aug 22, 2024

rcomer Aug 25, 2024

story645 Aug 25, 2024

story645 Aug 22, 2024

rcomer Aug 25, 2024

QuLogic commented Sep 12, 2024

rcomer commented Sep 17, 2024

	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.""",
	)

Search code, repositories, users, issues, pull requests...

Create InsetIndicator artist #27996

Create InsetIndicator artist #27996

Conversation

rcomer commented Mar 30, 2024 • edited Loading

PR summary

PR checklist

rcomer commented Mar 30, 2024

rcomer commented Jul 27, 2024

story645 commented Jul 28, 2024 • edited Loading

rcomer commented Jul 28, 2024

story645 commented Jul 28, 2024 • edited Loading

rcomer commented Aug 11, 2024 • edited Loading

rcomer commented Aug 12, 2024

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

rcomer commented Aug 14, 2024

tacaswell commented Aug 15, 2024

story645 commented Aug 15, 2024

jklymak commented Aug 16, 2024

rcomer commented Aug 16, 2024

story645 commented Aug 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

story645 Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuLogic commented Sep 12, 2024

rcomer commented Sep 17, 2024

Create `InsetIndicator` artist #27996

Create `InsetIndicator` artist #27996

rcomer commented Mar 30, 2024 •

edited

Loading

story645 commented Jul 28, 2024 •

edited

Loading

story645 commented Jul 28, 2024 •

edited

Loading

rcomer commented Aug 11, 2024 •

edited

Loading

story645 Aug 22, 2024 •

edited

Loading