Drawn ROIs affect legend placement in matplotlib 3.7

[Carifio24]

Describe the bug
With matplotlib 3.7 (in particular the updates from matplotlib/matplotlib#9598), ROIs drawn with selection tools now affect the placement of the legend on the graph when using the "draggable" (default) or "best" legend location options. The reason for this is that the path of non-rectangular patches is now used when determining the best option. This means that the Patch objects drawn during ROI selection affect the legend placement (even though they aren't visible). Note that some of the patches have an initial size (e.g. the patch for MplPolygonalROI has initial vertices [0, 0] and [1, 1]) which can cause this issue to manifest before any selections are made, depending on the viewer bounds.

To Reproduce
Steps to reproduce the behavior such as:

1. Open up glue
2. Add some data to a scatter plot and enable the legend
3. Wherever the legend shows up, make an ROI selection such that the ROI patch overlaps the legend (which points are selected doesn't matter).
4. The legend will likely move

Expected behavior
Selection ROIs shouldn't affect legend placement.

Screenshots
Here's an example screenshot. Matplotlib prefers the center-right placement of the legend, which overlaps a visible data point, over the open top-right area, because I had made a selection with a rectangular ROI there.

Details:

• Operating System: Ubuntu 20.04
• Python version (python --version): 3.10.9
• Glue version (glue --version): 1.7.0
• How you installed glue: pip

Additional context
Note that this is the cause of four of the five current CI failures (see #2366 for a resolution to the other one). The discrepancy in the tests occurs because the patches aren't included in the export script.

[Carifio24]

One solution to this problem could be to only add the patch to the axes during the ROI's start_selection and remove it during finalize_selection rather than changing the visibility. I'm not sure how much of an effect, if at all, this would have on performance.

[dhomeier]

Selection ROIs shouldn't affect legend placement.

Not quite sure if I actually agree with this. If an ROI exists when plotting, one may reasonably expect that anything inside it is of some interest (that's in the name at least ;-), so it makes sense to avoid covering that area, if possible.
That's different of course if the ROI has been initialised in some way (semi-)automatically, without the user being aware of it – I could not track down yet why and how in the failing tests any ROIs would be present when creating the expected plots.

[Carifio24]

one may reasonably expect that anything inside it is of some interest (that's in the name at least ;-), so it makes sense to avoid covering that area, if possible.

That's true, the user has selected the ROI region for some purpose. I guess it just feels odd to me to be willing to draw the legend over points rather than an area that is visually empty.

I could not track down yet why and how in the failing tests any ROIs would be present when creating the expected plots.

There aren't any ROIs that have been created yet, but I think the issue with the failing tests is due to the Patch object associated with the polygonal selection. This patch is created with points (0,0) and (1,1). With the unit-square boundaries that we have in the test cases, this is a diagonal line from the bottom-left to top-right that goes almost the whole way across the viewer. Displaying this polygon's path, which is what's used in the legend placement algorithm, gives the screenshot below. (Note that when determining the legend placement, matplotlib transforms everything into pixel space, so it's the transformed path that gets used, but the effect is identical).

[dhomeier]

And that is created, I assume, with ScatterViewers instantiation of tools: [..., 'select:polygon']; this would be consistent with the 'expected' plots all having the legend moved to the upper right.
Initialising MplPolygonalROI instead with a shorter polygon (or even a fully degenerate zip([0.5, 0.5], [0.5, 0.5])) lets the tests pass, but I agree that a cleaner solution would be to delay the patch synchronisation until a "real" selection has been made.

[Carifio24]

This was resolved with #2370 and can be closed.