Skip to content
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

Jump to bottom

Adding support for rotating mpl selectors of Ellipse,RectanglePixelRegion #390

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Adding support for rotating mpl selectors of Ellipse,RectanglePixelRegion #390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
327d327
Support rotation of Ellipse,RectanglePixelRegion MPL selectors
dhomeier Jul 29, 2021
2e72595
TST: Update `test_as_mpl_selector` to accept rotated regions
dhomeier Jul 29, 2021
0dce5a8
Switch to mpl #20839 rotation implementation
dhomeier Feb 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ New Features

- Added the DS9 'boxcircle' point symbol. [#387]

- Enable rotation of the ``as_mpl_selector`` widgets for rectangular
and ellipse regions with matplotlib versions supporting this. [#390]

Copy link
Member

Choose a reason for hiding this comment

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

This will need to be be moved to version 0.8.

- Added the ability to add and subtract ``PixCoord`` objects. [#396]

- Added an ``origin`` keyword to ``PolygonPixelRegion`` to allow
Expand Down
9 changes: 5 additions & 4 deletions regions/_utils/optional_deps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
try:
import matplotlib
HAS_MATPLOTLIB = True
MPL_VERSION = getattr(matplotlib, '__version__', None)
if MPL_VERSION is None:
MPL_VERSION = matplotlib._version.version
MPL_VERSION = MPL_VERSION.split('.')
MPL_VER_STR = getattr(matplotlib, '__version__', None)
if MPL_VER_STR is None:
MPL_VER_STR = matplotlib._version.version
MPL_VERSION = MPL_VER_STR.split('.')
MPL_VERSION = 10 * int(MPL_VERSION[0]) + int(MPL_VERSION[1])
except ImportError:
HAS_MATPLOTLIB = False
MPL_VER_STR = 'None'
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to streamline the matplotilb version checking? Something like what astropy uses for numpy version checks (see https://github.com/astropy/astropy/blob/main/astropy/utils/compat/numpycompat.py)?

from astropy.utils import minversion

MPL_LT_3_6 = not minversion(matplotlib, '3.6')

and then use it the code with:

if MPL_LT_3_6:
  ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing; will look into it.
Want to check first if this can be made to work with matplotlib/matplotlib#21945, which I've just reopened as matplotlib/matplotlib#26833 – unfortunately switching to display coordinates seems to break not just rotation but our existing resize and move operations; getting errors about self.height = ymax - ymin in _update_from_mpl_selector being set to negative values...

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. Let me know when this is ready for a (final?) review.

MPL_VERSION = 0
41 changes: 27 additions & 14 deletions regions/shapes/ellipse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,13 +212,23 @@ def as_artist(self, origin=(0, 0), **kwargs):
**mpl_kwargs)

def _update_from_mpl_selector(self, *args, **kwargs):
# _rect_properties replace _rect_bbox in matplotlib#19864, unchanged in #20839.
# "Note that if rotation != 0, ``xmin, ymin`` are always interpreted as the
# lower corner, and ``xmax, ymax`` are calculated using only width and
# height assuming no rotation (as specified for ``selector.extents``)."

xmin, xmax, ymin, ymax = self._mpl_selector.extents
self.center = PixCoord(x=0.5 * (xmin + xmax),
y=0.5 * (ymin + ymax))
self.width = (xmax - xmin)
self.height = (ymax - ymin)
self.angle = 0. * u.deg
if self._mpl_selector_callback is not None:
self.width = xmax - xmin
self.height = ymax - ymin
if hasattr(self._mpl_selector, 'rotation'):
rotation = self._mpl_selector.rotation
self.center = PixCoord(*self._mpl_selector.center)
else:
self.center = PixCoord(x=0.5 * (xmin + xmax), y=0.5 * (ymin + ymax))
rotation = 0
self.angle = rotation * u.deg

if getattr(self, '_mpl_selector_callback', None) is not None:
self._mpl_selector_callback(self)

def as_mpl_selector(self, ax, active=True, sync=True, callback=None,
Expand Down Expand Up @@ -264,14 +274,14 @@ def as_mpl_selector(self, ax, active=True, sync=True, callback=None,
``selector.set_active(True)`` or ``selector.set_active(False)``.
"""
from matplotlib.widgets import EllipseSelector

from regions._utils.optional_deps import MPL_VERSION
from regions._utils.optional_deps import MPL_VERSION, MPL_VER_STR

if hasattr(self, '_mpl_selector'):
raise AttributeError('Cannot attach more than one selector to a region.')

if self.angle.value != 0:
raise NotImplementedError('Cannot create matplotlib selector for rotated ellipse.')
if self.angle.value != 0 and not hasattr(EllipseSelector, 'rotation'):
raise NotImplementedError('Creating selectors for rotated shapes is not '
f'yet supported with matplotlib {MPL_VER_STR}.')

if sync:
sync_callback = self._update_from_mpl_selector
Expand All @@ -292,10 +302,13 @@ def sync_callback(*args, **kwargs):

self._mpl_selector = EllipseSelector(ax, sync_callback, interactive=True, **kwargs)

self._mpl_selector.extents = (self.center.x - self.width / 2,
self.center.x + self.width / 2,
self.center.y - self.height / 2,
self.center.y + self.height / 2)
xy0 = [self.center.x - self.width / 2, self.center.y - self.height / 2]
self._mpl_selector.extents = (xy0[0], self.center.x + self.width / 2,
xy0[1], self.center.y + self.height / 2)

if self.angle.value != 0:
self._mpl_selector.rotation = self.angle.to_value('deg')

self._mpl_selector.set_active(active)
self._mpl_selector_callback = callback

Expand Down
42 changes: 27 additions & 15 deletions regions/shapes/rectangle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,23 @@ def as_artist(self, origin=(0, 0), **kwargs):
angle=angle, **mpl_kwargs)

def _update_from_mpl_selector(self, *args, **kwargs):
# _rect_properties replace _rect_bbox in matplotlib#19864, unchanged in #20839.
# "Note that if rotation != 0, ``xmin, ymin`` are always interpreted as the
# lower corner, and ``xmax, ymax`` are calculated using only width and
# height assuming no rotation (as specified for ``selector.extents``)."

xmin, xmax, ymin, ymax = self._mpl_selector.extents
self.center = PixCoord(x=0.5 * (xmin + xmax),
y=0.5 * (ymin + ymax))
self.width = (xmax - xmin)
self.height = (ymax - ymin)
self.angle = 0. * u.deg
if self._mpl_selector_callback is not None:
self.width = xmax - xmin
self.height = ymax - ymin
if hasattr(self._mpl_selector, 'rotation'):
rotation = self._mpl_selector.rotation
self.center = PixCoord(*self._mpl_selector.center)
else:
self.center = PixCoord(x=0.5 * (xmin + xmax), y=0.5 * (ymin + ymax))
rotation = 0
self.angle = rotation * u.deg

if getattr(self, '_mpl_selector_callback', None) is not None:
self._mpl_selector_callback(self)

def as_mpl_selector(self, ax, active=True, sync=True, callback=None,
Expand Down Expand Up @@ -260,15 +270,14 @@ def as_mpl_selector(self, ax, active=True, sync=True, callback=None,
``selector.set_active(True)`` or ``selector.set_active(False)``.
"""
from matplotlib.widgets import RectangleSelector

from regions._utils.optional_deps import MPL_VERSION
from regions._utils.optional_deps import MPL_VERSION, MPL_VER_STR

if hasattr(self, '_mpl_selector'):
raise AttributeError('Cannot attach more than one selector to a region.')

if self.angle.value != 0:
raise NotImplementedError('Cannot create matplotlib selector for '
'rotated rectangle.')
if self.angle.value != 0 and not hasattr(RectangleSelector, 'rotation'):
raise NotImplementedError('Creating selectors for rotated shapes is not '
f'yet supported with matplotlib {MPL_VER_STR}.')

if sync:
sync_callback = self._update_from_mpl_selector
Expand All @@ -289,10 +298,13 @@ def sync_callback(*args, **kwargs):

self._mpl_selector = RectangleSelector(ax, sync_callback, interactive=True, **kwargs)

self._mpl_selector.extents = (self.center.x - self.width / 2,
self.center.x + self.width / 2,
self.center.y - self.height / 2,
self.center.y + self.height / 2)
dxy = [self.width / 2, self.height / 2]
self._mpl_selector.extents = (self.center.x - dxy[0], self.center.x + dxy[0],
self.center.y - dxy[1], self.center.y + dxy[1])

if self.angle.value != 0:
self._mpl_selector.rotation = self.angle.to_value('deg')

self._mpl_selector.set_active(active)
self._mpl_selector_callback = callback

Expand Down
38 changes: 19 additions & 19 deletions regions/shapes/tests/test_ellipse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,20 @@ def update_mask(reg):
# For now this will only work with unrotated ellipses. Once this
# works with rotated ellipses, the following exception check can
# be removed as well as the ``angle=0 * u.deg`` in the call to
# copy() below.
with pytest.raises(NotImplementedError,
match=('Cannot create matplotlib selector for rotated ellipse.')):
self.reg.as_mpl_selector(ax)
# copy() below - should (hopefully) be implemented with mpl 3.6.
expected = [8.3, 4.9, 2.0, 1.0]
if MPL_VERSION < 36:
with pytest.raises(NotImplementedError,
match='Creating selectors for rotated shapes is not yet supported'):
self.reg.as_mpl_selector(ax)
angle = 0 * u.deg
else:
angle = self.reg.angle

region = self.reg.copy(angle=0 * u.deg)
if not sync:
expected = [3, 4, 4, 3]

region = self.reg.copy(angle=angle)

selector = region.as_mpl_selector(ax, callback=update_mask, sync=sync)

Expand All @@ -144,24 +152,16 @@ def update_mask(reg):

ax.figure.canvas.draw()

if sync:
assert_allclose(region.center.x, expected[0])
assert_allclose(region.center.y, expected[1])
assert_allclose(region.width, expected[2])
assert_allclose(region.height, expected[3])

assert_allclose(region.center.x, 8.3)
assert_allclose(region.center.y, 4.9)
assert_allclose(region.width, 2)
assert_allclose(region.height, 1)
if sync:
assert_quantity_allclose(region.angle, 0 * u.deg)

assert_equal(mask, region.to_mask(mode='subpixels', subpixels=10).to_image(data.shape))

else:

assert_allclose(region.center.x, 3)
assert_allclose(region.center.y, 4)
assert_allclose(region.width, 4)
assert_allclose(region.height, 3)
assert_quantity_allclose(region.angle, 0 * u.deg)

assert_quantity_allclose(region.angle, angle)
assert_equal(mask, 0)

with pytest.raises(AttributeError, match=('Cannot attach more than one selector to a reg')):
Expand Down
37 changes: 20 additions & 17 deletions regions/shapes/tests/test_rectangle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,21 @@ def update_mask(reg):
# For now this will only work with unrotated rectangles. Once
# this works with rotated rectangles, the following exception
# check can be removed as well as the ``angle=0 * u.deg`` in the
# call to copy() below.
with pytest.raises(NotImplementedError,
match=('Cannot create matplotlib selector for rotated rectangle.')):
self.reg.as_mpl_selector(ax)
# copy() below - should (hopefully) be implemented with mpl 3.6.
expected = [8.3, 4.9, 2.0, 1.0]
if MPL_VERSION < 36:
with pytest.raises(NotImplementedError,
match='Creating selectors for rotated shapes is not yet supported'):
self.reg.as_mpl_selector(ax)

angle = 0 * u.deg
else:
angle = self.reg.angle

region = self.reg.copy(angle=0 * u.deg)
if not sync:
expected = [3, 4, 4, 3]

region = self.reg.copy(angle=angle)

selector = region.as_mpl_selector(ax, callback=update_mask, sync=sync)

Expand All @@ -150,22 +159,16 @@ def update_mask(reg):

ax.figure.canvas.draw()

assert_allclose(region.center.x, expected[0])
assert_allclose(region.center.y, expected[1])
assert_allclose(region.width, expected[2])
assert_allclose(region.height, expected[3])

if sync:
assert_allclose(region.center.x, 8.3)
assert_allclose(region.center.y, 4.9)
assert_allclose(region.width, 2)
assert_allclose(region.height, 1)
assert_quantity_allclose(region.angle, 0 * u.deg)

assert_equal(mask, region.to_mask(mode='subpixels', subpixels=10).to_image(data.shape))

else:
assert_allclose(region.center.x, 3)
assert_allclose(region.center.y, 4)
assert_allclose(region.width, 4)
assert_allclose(region.height, 3)
assert_quantity_allclose(region.angle, 0 * u.deg)

assert_quantity_allclose(region.angle, angle)
assert_equal(mask, 0)

with pytest.raises(AttributeError, match=('Cannot attach more than one selector to a reg')):
Expand Down