From 12715d67982f718e7d3d72d33cf034f700056611 Mon Sep 17 00:00:00 2001 From: Clare Shanahan Date: Thu, 5 Oct 2023 23:21:57 -0400 Subject: [PATCH 1/4] test and docstring updates for `roi_subset_state_to_region` --- glue_astronomy/translators/regions.py | 21 +++-- .../translators/tests/test_regions.py | 79 ++++++++++++++----- 2 files changed, 77 insertions(+), 23 deletions(-) diff --git a/glue_astronomy/translators/regions.py b/glue_astronomy/translators/regions.py index a9595ab..dbc909b 100644 --- a/glue_astronomy/translators/regions.py +++ b/glue_astronomy/translators/regions.py @@ -56,9 +56,19 @@ def range_to_rect(data, ori, low, high): def roi_subset_state_to_region(subset_state, to_sky=False): """Translate the given ``RoiSubsetState`` containing ROI that is compatible with 2D spatial regions to proper - ``regions`` shape. If ``to_sky=True`` is given, it will - return sky region from attached data WCS, otherwise it returns - pixel region. + ``regions`` shape. + + If ``to_sky`` is False, it will return the region in pixel coordinates. + If ``to_sky=True``, it will return the region transformed to sky + coordinates, per attached data WCS. + + Parameters + ---------- + subset_state : `~glue.core.subset.SubsetState` + ROI subset state. + to_sky: bool + If True, return region in celestial coordinates from attached data WCS. + (Default=False). """ roi = subset_state.roi @@ -92,9 +102,10 @@ def roi_subset_state_to_region(subset_state, to_sky=False): else: raise NotImplementedError(f"ROIs of type {roi.__class__.__name__} are not yet supported") - if to_sky: + if to_sky is True: + if subset_state.xatt.parent.coords is None: + raise ValueError("No WCS associated with subset data, can't do to_sky transformation.") reg = reg.to_sky(subset_state.xatt.parent.coords) - return reg diff --git a/glue_astronomy/translators/tests/test_regions.py b/glue_astronomy/translators/tests/test_regions.py index bcad68d..4b5bc5f 100644 --- a/glue_astronomy/translators/tests/test_regions.py +++ b/glue_astronomy/translators/tests/test_regions.py @@ -1,14 +1,15 @@ import pytest import numpy as np from astropy import units as u +from astropy.wcs import WCS from astropy.tests.helper import assert_quantity_allclose from numpy.testing import assert_allclose, assert_array_equal, assert_equal from packaging.version import Version -from regions import (RectanglePixelRegion, RectangleSkyRegion, - PolygonPixelRegion, CirclePixelRegion, - EllipsePixelRegion, PointPixelRegion, CompoundPixelRegion, - CircleAnnulusPixelRegion, PixCoord) +from regions import (RectanglePixelRegion, RectangleSkyRegion, PolygonPixelRegion, + CirclePixelRegion, CircleSkyRegion, + EllipsePixelRegion, PointPixelRegion, PointSkyRegion, + CompoundPixelRegion, CircleAnnulusPixelRegion, PixCoord) from glue.core import Data, DataCollection from glue.core.roi import (RectangularROI, PolygonalROI, CircularROI, EllipticalROI, @@ -32,11 +33,31 @@ def setup_method(self, method): self.data.coords = WCS_CELESTIAL self.dc = DataCollection([self.data]) + def setup_rois(self, roi_shape_name, *args, without_wcs=False): + # define different ROI shapes that are used in several tests. + + data = self.data + + if without_wcs: + data = Data(flux=np.ones((128, 256))) # define Data w/o WCS + + if roi_shape_name == 'CircularROI': + roi = CircularROI(*args) + if roi_shape_name == 'RectangularROI': + roi = RectangularROI(*args) + if roi_shape_name == 'PointROI': + roi = PointROI(*args) + if roi_shape_name == 'PolygonalROI': + roi = PolygonalROI(*args) + + return RoiSubsetState(data.pixel_component_ids[1], + data.pixel_component_ids[0], + roi) + def test_rectangular_roi(self): - subset_state = RoiSubsetState(self.data.pixel_component_ids[1], - self.data.pixel_component_ids[0], - RectangularROI(1, 3.5, -0.2, 3.3)) + subset_state = self.setup_rois('RectangularROI', + *[1, 3.5, -0.2, 3.3]) self.dc.new_subset_group(subset_state=subset_state, label='rectangular') @@ -49,17 +70,14 @@ def test_rectangular_roi(self): assert_allclose(reg.width, 2.5) assert_allclose(reg.height, 3.5) - reg_sky = roi_subset_state_to_region(subset_state, to_sky=True) - assert isinstance(reg_sky, RectangleSkyRegion) def test_polygonal_roi(self): xv = [1.3, 2, 3, 1.5, 0.5] yv = [10, 20.20, 30, 25, 17.17] - subset_state = RoiSubsetState(self.data.pixel_component_ids[1], - self.data.pixel_component_ids[0], - PolygonalROI(xv, yv)) + subset_state = self.setup_rois('PolygonalROI', + *[xv, yv]) self.dc.new_subset_group(subset_state=subset_state, label='polygon') @@ -72,9 +90,7 @@ def test_polygonal_roi(self): def test_circular_roi(self): - subset_state = RoiSubsetState(self.data.pixel_component_ids[1], - self.data.pixel_component_ids[0], - CircularROI(1, 3.5, 0.75)) + subset_state = self.setup_rois('CircularROI', *[1, 3.5, 0.75]) self.dc.new_subset_group(subset_state=subset_state, label='circular') @@ -113,9 +129,7 @@ def test_ellipse_roi(self, theta): def test_point_roi(self): - subset_state = RoiSubsetState(self.data.pixel_component_ids[1], - self.data.pixel_component_ids[0], - PointROI(2.64, 5.4)) + subset_state = self.setup_rois('PointROI', *[2.64, 5.4]) self.dc.new_subset_group(subset_state=subset_state, label='point') @@ -488,3 +502,32 @@ def test_unsupported(self): match='Subset states of type InequalitySubsetState are not supported'): self.data.get_selection_definition(format='astropy-regions', subset_id='Flux-based selection') + + @pytest.mark.parametrize("subset_state_name, output_pixel_shape, output_sky_shape, args", + [('CircularROI', CirclePixelRegion, CircleSkyRegion, [1, 3.5, 0.75]), + ('RectangularROI', RectanglePixelRegion, RectangleSkyRegion, [0, 2, 0, 7]), + ('PointROI', PointPixelRegion, PointSkyRegion, [1, 3.5])]) + def test_roi_subset_state_to_region(self, subset_state_name, output_pixel_shape, + output_sky_shape, args): + # test returning pixel/sky regions with `roi_subset_state_to_region` + # parameterized over multiple shapes with various attributes, so just + # check transformation of center coordinates. + + subset_state = self.setup_rois(subset_state_name, *args) + + # test that only pixel region is returned when to_sky is False + reg_pixel = roi_subset_state_to_region(subset_state, to_sky=False) + assert isinstance(reg_pixel, output_pixel_shape) + assert_allclose(reg_pixel.center.x, 1) + assert_allclose(reg_pixel.center.y, 3.5) + + # test sky region + reg_sky = roi_subset_state_to_region(subset_state, to_sky=True) + assert isinstance(reg_sky, output_sky_shape) + assert_allclose(reg_sky.center.ra.deg, 1.99918828) + assert_allclose(reg_sky.center.dec.deg, 4.48805907) + + # and that proper error is raised when there is no WCS + with pytest.raises(ValueError, match="No WCS associated with subset data, can't do to_sky transformation."): + subset_state = self.setup_rois(subset_state_name, *args, without_wcs=True) + roi_subset_state_to_region(subset_state, to_sky=True) From 50cb2a2b11551aeb2f2c59814a3cf1fa0c283ff6 Mon Sep 17 00:00:00 2001 From: Clare Shanahan Date: Wed, 11 Oct 2023 16:02:08 -0400 Subject: [PATCH 2/4] removed unused import --- glue_astronomy/translators/tests/test_regions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/glue_astronomy/translators/tests/test_regions.py b/glue_astronomy/translators/tests/test_regions.py index 4b5bc5f..51e78ed 100644 --- a/glue_astronomy/translators/tests/test_regions.py +++ b/glue_astronomy/translators/tests/test_regions.py @@ -1,7 +1,6 @@ import pytest import numpy as np from astropy import units as u -from astropy.wcs import WCS from astropy.tests.helper import assert_quantity_allclose from numpy.testing import assert_allclose, assert_array_equal, assert_equal from packaging.version import Version From 4550a35523aa14d71a6458bec36d7377195f7583 Mon Sep 17 00:00:00 2001 From: Derek Homeier <709020+dhomeier@users.noreply.github.com> Date: Tue, 17 Oct 2023 17:01:54 +0200 Subject: [PATCH 3/4] Fix codestyle errors --- glue_astronomy/translators/tests/test_regions.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/glue_astronomy/translators/tests/test_regions.py b/glue_astronomy/translators/tests/test_regions.py index 51e78ed..cd4f1b7 100644 --- a/glue_astronomy/translators/tests/test_regions.py +++ b/glue_astronomy/translators/tests/test_regions.py @@ -69,7 +69,6 @@ def test_rectangular_roi(self): assert_allclose(reg.width, 2.5) assert_allclose(reg.height, 3.5) - def test_polygonal_roi(self): xv = [1.3, 2, 3, 1.5, 0.5] @@ -503,9 +502,10 @@ def test_unsupported(self): subset_id='Flux-based selection') @pytest.mark.parametrize("subset_state_name, output_pixel_shape, output_sky_shape, args", - [('CircularROI', CirclePixelRegion, CircleSkyRegion, [1, 3.5, 0.75]), - ('RectangularROI', RectanglePixelRegion, RectangleSkyRegion, [0, 2, 0, 7]), - ('PointROI', PointPixelRegion, PointSkyRegion, [1, 3.5])]) + [('CircularROI', CirclePixelRegion, CircleSkyRegion, [1, 3.5, 0.75]), + ('RectangularROI', RectanglePixelRegion, RectangleSkyRegion, + [0, 2, 0, 7]), + ('PointROI', PointPixelRegion, PointSkyRegion, [1, 3.5])]) def test_roi_subset_state_to_region(self, subset_state_name, output_pixel_shape, output_sky_shape, args): # test returning pixel/sky regions with `roi_subset_state_to_region` @@ -527,6 +527,7 @@ def test_roi_subset_state_to_region(self, subset_state_name, output_pixel_shape, assert_allclose(reg_sky.center.dec.deg, 4.48805907) # and that proper error is raised when there is no WCS - with pytest.raises(ValueError, match="No WCS associated with subset data, can't do to_sky transformation."): + with pytest.raises(ValueError, match="No WCS associated with subset data, " + "can't do to_sky transformation."): subset_state = self.setup_rois(subset_state_name, *args, without_wcs=True) roi_subset_state_to_region(subset_state, to_sky=True) From aa305d7fc92caf714b55649a453608410d6bbe09 Mon Sep 17 00:00:00 2001 From: Derek Homeier <709020+dhomeier@users.noreply.github.com> Date: Fri, 20 Oct 2023 15:45:37 +0200 Subject: [PATCH 4/4] micro-formatting fix --- glue_astronomy/translators/regions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glue_astronomy/translators/regions.py b/glue_astronomy/translators/regions.py index dbc909b..32d1f97 100644 --- a/glue_astronomy/translators/regions.py +++ b/glue_astronomy/translators/regions.py @@ -66,7 +66,7 @@ def roi_subset_state_to_region(subset_state, to_sky=False): ---------- subset_state : `~glue.core.subset.SubsetState` ROI subset state. - to_sky: bool + to_sky : bool If True, return region in celestial coordinates from attached data WCS. (Default=False). """