From 04a0cfc2054907168dccf69e9e6a2d9cbd9776ed Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Mon, 24 Apr 2023 18:45:57 -0400 Subject: [PATCH 1/4] Add translator for CircularPixelAnnulus --- glue_astronomy/translators/regions.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/glue_astronomy/translators/regions.py b/glue_astronomy/translators/regions.py index 82000b9..7f6c54d 100644 --- a/glue_astronomy/translators/regions.py +++ b/glue_astronomy/translators/regions.py @@ -1,13 +1,13 @@ from glue.config import subset_state_translator from glue.core.subset import (RoiSubsetState, RangeSubsetState, OrState, AndState, - XorState, MultiOrState, Subset, MultiRangeSubsetState) + XorState, MultiOrState, Subset, MultiRangeSubsetState, InvertState) from glue.core.roi import (RectangularROI, PolygonalROI, CircularROI, PointROI, RangeROI, AbstractMplRoi, EllipticalROI) from glue.viewers.image.pixel_selection_subset_state import PixelSubsetState from astropy import units as u from regions import (RectanglePixelRegion, PolygonPixelRegion, CirclePixelRegion, - PointPixelRegion, PixCoord, EllipsePixelRegion) + PointPixelRegion, PixCoord, EllipsePixelRegion, CircleAnnulusPixelRegion) def range_to_rect(data, ori, low, high): @@ -132,11 +132,23 @@ def to_object(self, subset): return PointPixelRegion(PixCoord(*subset_state.get_xy(data, 1, 0))) elif isinstance(subset_state, AndState): - temp_sub1 = Subset(data=data) - temp_sub1.subset_state = subset_state.state1 - temp_sub2 = Subset(data=data) - temp_sub2.subset_state = subset_state.state2 - return self.to_object(temp_sub1) & self.to_object(temp_sub2) + if ((not isinstance(subset_state.state1, InvertState)) and + isinstance(subset_state.state1.roi, CircularROI) and + isinstance(subset_state.state2, InvertState) and + isinstance(subset_state.state2.state1.roi, CircularROI) and + (subset_state.state1.roi.xc == subset_state.state2.state1.roi.xc) and + (subset_state.state1.roi.yc == subset_state.state2.state1.roi.yc) and + (subset_state.state1.roi.radius > subset_state.state2.state1.roi.radius)): + return CircleAnnulusPixelRegion( + center=PixCoord(x=subset_state.state1.roi.xc, y=subset_state.state1.roi.yc), + inner_radius=subset_state.state2.state1.roi.radius, + outer_radius=subset_state.state1.roi.radius) + else: + temp_sub1 = Subset(data=data) + temp_sub1.subset_state = subset_state.state1 + temp_sub2 = Subset(data=data) + temp_sub2.subset_state = subset_state.state2 + return self.to_object(temp_sub1) & self.to_object(temp_sub2) elif isinstance(subset_state, OrState): temp_sub1 = Subset(data=data) From 5ab59d0c3ed573eeb4bd8ac1f33869b8cf23ab35 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Tue, 25 Apr 2023 15:36:52 -0400 Subject: [PATCH 2/4] Address comments, add test --- glue_astronomy/translators/regions.py | 40 +++++++++++++++---- .../translators/tests/test_regions.py | 33 +++++++++++---- 2 files changed, 58 insertions(+), 15 deletions(-) diff --git a/glue_astronomy/translators/regions.py b/glue_astronomy/translators/regions.py index 7f6c54d..aa4c31c 100644 --- a/glue_astronomy/translators/regions.py +++ b/glue_astronomy/translators/regions.py @@ -9,6 +9,8 @@ from regions import (RectanglePixelRegion, PolygonPixelRegion, CirclePixelRegion, PointPixelRegion, PixCoord, EllipsePixelRegion, CircleAnnulusPixelRegion) +__all__ = ["range_to_rect", "AstropyRegionsHandler"] + def range_to_rect(data, ori, low, high): """ @@ -46,6 +48,36 @@ def range_to_rect(data, ori, low, high): return RectanglePixelRegion(PixCoord(xcen, ycen), width, height) +def _is_annulus(subset_state): + # subset_state.state1 = outer circle + # subset_state.state2 = inner circle + # subset_state.state2 is inverted, so we need its state1 + return ((not isinstance(subset_state.state1, InvertState)) and + isinstance(subset_state.state1.roi, CircularROI) and + isinstance(subset_state.state2, InvertState) and + isinstance(subset_state.state2.state1.roi, CircularROI) and + (subset_state.state1.roi.xc == subset_state.state2.state1.roi.xc) and + (subset_state.state1.roi.yc == subset_state.state2.state1.roi.yc) and + (subset_state.state1.roi.radius > subset_state.state2.state1.roi.radius)) + + +# Put this here because there is nowhere else to put it. +# https://github.com/glue-viz/glue/issues/2390 +def _annulus_to_subset_state(reg, data): + """CircleAnnulusPixelRegion to glue subset state.""" + # TODO: Add ellipse and rectangle support. + if not isinstance(reg, CircleAnnulusPixelRegion): # pragma: no cover + raise NotImplementedError(f"{reg} not supported") + + xcen = reg.center.x + ycen = reg.center.y + state1 = RoiSubsetState(data.pixel_component_ids[1], data.pixel_component_ids[0], + CircularROI(xcen, ycen, reg.outer_radius)) + state2 = RoiSubsetState(data.pixel_component_ids[1], data.pixel_component_ids[0], + CircularROI(xcen, ycen, reg.inner_radius)) + return AndState(state1, ~state2) + + @subset_state_translator('astropy-regions') class AstropyRegionsHandler: @@ -132,13 +164,7 @@ def to_object(self, subset): return PointPixelRegion(PixCoord(*subset_state.get_xy(data, 1, 0))) elif isinstance(subset_state, AndState): - if ((not isinstance(subset_state.state1, InvertState)) and - isinstance(subset_state.state1.roi, CircularROI) and - isinstance(subset_state.state2, InvertState) and - isinstance(subset_state.state2.state1.roi, CircularROI) and - (subset_state.state1.roi.xc == subset_state.state2.state1.roi.xc) and - (subset_state.state1.roi.yc == subset_state.state2.state1.roi.yc) and - (subset_state.state1.roi.radius > subset_state.state2.state1.roi.radius)): + if _is_annulus(subset_state): return CircleAnnulusPixelRegion( center=PixCoord(x=subset_state.state1.roi.xc, y=subset_state.state1.roi.yc), inner_radius=subset_state.state2.state1.roi.radius, diff --git a/glue_astronomy/translators/tests/test_regions.py b/glue_astronomy/translators/tests/test_regions.py index 7ee3767..31249da 100644 --- a/glue_astronomy/translators/tests/test_regions.py +++ b/glue_astronomy/translators/tests/test_regions.py @@ -6,7 +6,8 @@ from packaging.version import Version from regions import (RectanglePixelRegion, PolygonPixelRegion, CirclePixelRegion, - EllipsePixelRegion, PointPixelRegion, CompoundPixelRegion, PixCoord) + EllipsePixelRegion, PointPixelRegion, CompoundPixelRegion, + CircleAnnulusPixelRegion, PixCoord) from glue.core import Data, DataCollection from glue.core.roi import (RectangularROI, PolygonalROI, CircularROI, EllipticalROI, @@ -18,11 +19,13 @@ from glue.viewers.image.pixel_selection_subset_state import PixelSubsetState from glue import __version__ as glue_version +from glue_astronomy.translators.regions import _annulus_to_subset_state + class TestAstropyRegions: def setup_method(self, method): - self.data = Data(flux=np.ones((128, 256))) + self.data = Data(flux=np.ones((128, 256))) # ny, nx self.dc = DataCollection([self.data]) def test_rectangular_roi(self): @@ -304,6 +307,20 @@ def test_and_region(self): assert not reg.contains(PixCoord(5.1, 6.1)) assert not reg.contains(PixCoord(11, 12)) + def test_circular_annulus(self): + reg_orig = CircleAnnulusPixelRegion( + center=PixCoord(x=50, y=25), inner_radius=7, outer_radius=13) + subset_state = _annulus_to_subset_state(reg_orig, self.data) + self.dc.new_subset_group(subset_state=subset_state, label='annulus_1') + reg = self.data.get_selection_definition(subset_id='annulus_1', format='astropy-regions') + + # Would round-trip if translator worked correctly. + assert isinstance(reg, CircleAnnulusPixelRegion) + assert reg.center.x == reg_orig.center.x + assert reg.center.y == reg_orig.center.y + assert reg.outer_radius == reg_orig.outer_radius + assert reg.inner_radius == reg_orig.inner_radius + def test_or_region(self): subset_state1 = RoiSubsetState(self.data.pixel_component_ids[1], self.data.pixel_component_ids[0], @@ -394,7 +411,7 @@ def test_main_component_combos(self): multior_region = self.data.get_selection_definition(subset_id='multior', format='astropy-regions') - for reg in and_region, or_region, xor_region, multior_region: + for reg in (and_region, or_region, xor_region, multior_region): assert isinstance(reg, CompoundPixelRegion) assert isinstance(reg.region1, RectanglePixelRegion) assert isinstance(reg.region2, CirclePixelRegion) @@ -436,7 +453,7 @@ def test_subset_id(self): self.dc.new_subset_group(subset_state=subset_state, label='rectangular') - for subset_id in [None, 0, 'rectangular']: + for subset_id in (None, 0, 'rectangular'): reg = self.data.get_selection_definition(format='astropy-regions', subset_id=subset_id) assert isinstance(reg, RectanglePixelRegion) @@ -445,15 +462,15 @@ def test_subset_id(self): assert_allclose(reg.width, 2.5) assert_allclose(reg.height, 3.5) - with pytest.raises(ValueError) as exc: + with pytest.raises(ValueError, match="No subset found with the label 'circular'"): self.data.get_selection_definition(format='astropy-regions', subset_id='circular') - assert exc.value.args[0] == "No subset found with the label 'circular'" def test_unsupported(self): self.dc.new_subset_group(subset_state=self.data.id['flux'] > 0.5, label='Flux-based selection') - with pytest.raises(NotImplementedError) as exc: + with pytest.raises( + NotImplementedError, + match='Subset states of type InequalitySubsetState are not supported'): self.data.get_selection_definition(format='astropy-regions', subset_id='Flux-based selection') - assert exc.value.args[0] == 'Subset states of type InequalitySubsetState are not supported' From dc079bb0aa32c2ea06d16031538f0b92b2c01cb8 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Wed, 26 Apr 2023 11:17:28 -0400 Subject: [PATCH 3/4] Make exception clearer in _annulus_to_subset_state Co-authored-by: Derek Homeier --- glue_astronomy/translators/regions.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/glue_astronomy/translators/regions.py b/glue_astronomy/translators/regions.py index aa4c31c..880707d 100644 --- a/glue_astronomy/translators/regions.py +++ b/glue_astronomy/translators/regions.py @@ -64,9 +64,11 @@ def _is_annulus(subset_state): # Put this here because there is nowhere else to put it. # https://github.com/glue-viz/glue/issues/2390 def _annulus_to_subset_state(reg, data): - """CircleAnnulusPixelRegion to glue subset state.""" - # TODO: Add ellipse and rectangle support. - if not isinstance(reg, CircleAnnulusPixelRegion): # pragma: no cover + """AnnulusPixelRegion to glue subset state.""" + if not isinstance(reg, AnnulusPixelRegion): + raise ValueError("`reg` should be an `AnnulusPixelRegion` instance") + # TODO: Add ellipse and rectangle annulus support. + elif not isinstance(reg, CircleAnnulusPixelRegion): # pragma: no cover raise NotImplementedError(f"{reg} not supported") xcen = reg.center.x From 6ca48f61f724a307248d8c93a5ad2b145b21f781 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Wed, 26 Apr 2023 11:20:13 -0400 Subject: [PATCH 4/4] Fix suggestion --- glue_astronomy/translators/regions.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/glue_astronomy/translators/regions.py b/glue_astronomy/translators/regions.py index 880707d..ffffa28 100644 --- a/glue_astronomy/translators/regions.py +++ b/glue_astronomy/translators/regions.py @@ -7,7 +7,8 @@ from astropy import units as u from regions import (RectanglePixelRegion, PolygonPixelRegion, CirclePixelRegion, - PointPixelRegion, PixCoord, EllipsePixelRegion, CircleAnnulusPixelRegion) + PointPixelRegion, PixCoord, EllipsePixelRegion, + AnnulusPixelRegion, CircleAnnulusPixelRegion) __all__ = ["range_to_rect", "AstropyRegionsHandler"] @@ -65,10 +66,10 @@ def _is_annulus(subset_state): # https://github.com/glue-viz/glue/issues/2390 def _annulus_to_subset_state(reg, data): """AnnulusPixelRegion to glue subset state.""" - if not isinstance(reg, AnnulusPixelRegion): - raise ValueError("`reg` should be an `AnnulusPixelRegion` instance") + if not isinstance(reg, AnnulusPixelRegion): # pragma: no cover + raise ValueError(f"{reg} is not an AnnulusPixelRegion instance") # TODO: Add ellipse and rectangle annulus support. - elif not isinstance(reg, CircleAnnulusPixelRegion): # pragma: no cover + if not isinstance(reg, CircleAnnulusPixelRegion): # pragma: no cover raise NotImplementedError(f"{reg} not supported") xcen = reg.center.x