Improve RectangleSelector rotation

[dstansby]

PR Summary

This modifies RectangleSelector and EllipseSelector to be drawn in display coordinates instead of display coordinates. This has a number of advantages:

• Allows the rectangle to be rotated past the previous +/- 45 degrees limit
• Fixes scaling the rectangle when it is rotated (to demonstrate, create a selector with rotation and try scaling before and then with this PR).
• Removes the need for _aspect_ratio_correction.
• Improves performance, because the event handling and artist drawing is all done in figure coordinates, removing transformations to data coordinates and back.

Code to test:

import matplotlib.pyplot as plt
from matplotlib.widgets import RectangleSelector


fig = plt.figure()
ax = fig.add_subplot()

selector = RectangleSelector(ax, lambda *args: None, interactive=True)
selector.extents = (0.4, 0.7, 0.3, 0.4)
selector.add_state('rotate')

plt.show()

Fixes #21937

PR Checklist

Tests and Styling

• Has pytest style unit tests (and pytest passes).
• Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

• New features are documented, with examples if plot related.
• New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
• API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
• Documentation is sphinx and numpydoc compliant (the docs should build without error).

[ericpre]

This looks very promising!

[dstansby]

This is ready for review!

[jklymak]

Confused about the status here - is it waiting, or is it ready? I'll assume waiting and set to draft, but feel free to pop back on queue.

[ericpre]

Best to sort out #21977 first before continuing on this PR.

[dhomeier]

I think it makes sense to keep the width/height fixed in display coords, and am undecided on whether a point (either the corner or center?) should be fixed in display or data coords.

Looking at it from the data side, so to speak, I would rather give preference to conserving area in data space – holding width and height fixed in data coords would of course be a sufficient, but perhaps not necessary condition for that.
Thinking in particular of the link to astropy/regions#390 here; I don't know if there are other applications in support of this, or if it really should get preference over implementing a better performing solution on the Matplotlib side.

[dstansby]

Looking at it from the data side, so to speak, I would rather give preference to conserving area in data space – holding width and height fixed in data coords would of course be a sufficient, but perhaps not necessary condition for that.

What are your thoughts in the instance where the selector is rotated, where it's impossible to keep the selector rectangular (ie. have right angles as corners in display coordinates) and conserve both width, height in data coordinates?

[dhomeier]

What are your thoughts in the instance where the selector is rotated, where it's impossible to keep the selector rectangular (ie. have right angles as corners in display coordinates) and conserve both width, height in data coordinates?

Intuitively it certainly makes interaction easier if angles and aspect ratio are conserved in display coordinates.
Just trying to figure out if the area in data coordinates is not automatically conserved after all – that certainly is the case for 45˚ or 90˚ rotations, as e.g. doubling one side in and aspect ratio of 2 would simply reduce the other by half.
And since the selector is no longer rectangular in data coordinates, the concept of width and height becomes ambiguous anyway – is it to be the parallelogram height or side length?

[jklymak]

@dstansby have you considered making a simpler "RotatingRectangleSelector" that is mostly meant for the fixed-aspect ratio case? I really don't see the point of rotating a rectangle if the data dimensions are different in x and y, so this is really mostly for the image processing case?. If you spin it, it can have whatever whacky behaviour it wants if the data aspect is not 1:1 without introducing confusing behaviour in the normal RectangleSelector?

[dstansby]

I am not adverse to doing that, but as it currently stands in the main branch (from #20839) it is currently possible to rotate a rectangle with axes aspect!=1 and the rectangle stays rectangular in display coordinates, not data coordinates. This PR removes limitations on that approach and fixes bugs with it.

I am happy to either:

• Carry on advocating for this PR
• Revert some of Fix centre and square state and add rotation for rectangle selector #20839 so that RectangleSelector is always rectangular in data coordinates (ie. may look like a trapezium in display coordinates), and create a different selector

Either will take significant effort though, so it would be good to know which of these options has support! Personally I think it makes sense to have the rectangle always rectangular in display coords (this PR), but am not against creating a new selector to do this.

[jklymak]

@ericpre @anntzer both of your names come to mind to comment on how to proceed here...

I can't see ever using this, so I don't have a strong opinion about what is supposed to happen if we rotate a rectangle selector in non one-to-one aspect space. It seems ill defined. Maybe the solution is to not offer that option if the aspect ratio is not 1:1?

[jklymak]

I added to the weekly meeting this week. However @dstansby if you would like to attend to discuss we can defer until you are available. Meetings are Thu 19:00 UTC : https://hackmd.io/49a-u44CTja02xQRaG88JA

[jklymak]

We discussed on the meeting, and the consensus that the only sane thing is for the rectangle to stay a rectangle in display coordinates. Anything else doesn't work in any sort of non-cartesian co-ordinate system.

It also seems that resizing the window, etc is a bit beyond the point. If you make a selection hopefully something happens with that selection before any other GUI actions are made. I guess that depends not he use case, but it seems that GUI rectangles can't be expected to respond to window resizing, and anything built on the widget should deal with it another way.

[dstansby]

thanks for discussing! I'm afraid I don't have the time to dig this up at the moment, as you can probably see it's quite a complex PR!

[tacaswell]

@dstansby What is the state / fate of this?

[dstansby]

I think the status is it should definitely go in, but the rebase needs handling, and the what's new and/or bugfixes probably need updating or adding.

I think one of @keflavich or @dhomeier might have some funding from astropy to get this into a mergeable state? Sorry if I'm wrong there, but if I'm right would one of you be happy picking this PR up?

[keflavich]

There is funding in astropy to do this, but I can't put in the effort. Someone from Aperio, possibly @dhomeier, may be able to but we haven't worked out details yet.

[dhomeier]

I am planning to work on this along with astropy/regions#390, but certainly won't be able to start before next year.