Repository: spacetelescope/jdaviz

Issue:
BUG: Ellipse translation in app.get_subsets() is wrong
https://github.com/spacetelescope/jdaviz/blob/7ffaa2691bb4cfd4efaece83177b1f171463579f/jdaviz/app.py#L1009

is wrong because `glue` defines *radius* while `regions` defines *width and height*. Really, we should be using the translators already vetted and in place here: https://github.com/glue-viz/glue-astronomy/blob/main/glue_astronomy/translators/regions.py

The complication is that to use the upstream translator directly the way we do the translation in this method, we also need to modify upstream.

I already have the fixes but I open an issue anyway so users are aware, because this bug is very subtle. Fixes here:

* https://github.com/glue-viz/glue-astronomy/pull/93
* https://github.com/spacetelescope/jdaviz/pull/2240

[🐱](https://jira.stsci.edu/browse/JDAT-3480)

Discussion:

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index be79e70c..379ce4b3 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -42,6 +42,8 @@ Specviz2d
 Bug Fixes
 ---------
 
+- Fixed wrong elliptical region translation in ``app.get_subsets()``. [#2244]
+
 Cubeviz
 ^^^^^^^
 
diff --git a/jdaviz/app.py b/jdaviz/app.py
index d2d3d9c2..f7c42c77 100644
--- a/jdaviz/app.py
+++ b/jdaviz/app.py
@@ -1006,7 +1006,7 @@ class Application(VuetifyTemplate, HubListener):
             rx = roi.radius_x
             ry = roi.radius_y
             theta = np.around(np.degrees(roi.theta), decimals=_around_decimals)
-            roi_as_region = EllipsePixelRegion(PixCoord(xc, yc), rx, ry, Angle(theta, "deg"))
+            roi_as_region = EllipsePixelRegion(PixCoord(xc, yc), rx * 2, ry * 2, Angle(theta, "deg"))  # noqa: E501
 
         return [{"name": subset_state.roi.__class__.__name__,
                  "glue_state": subset_state.__class__.__name__,


Test patch:
diff --git a/jdaviz/tests/test_subsets.py b/jdaviz/tests/test_subsets.py
index 76e8ef3b..bf4f14d9 100644
--- a/jdaviz/tests/test_subsets.py
+++ b/jdaviz/tests/test_subsets.py
@@ -332,7 +332,7 @@ def test_composite_region_from_subset_3d(cubeviz_helper):
     viewer.apply_roi(EllipticalROI(30, 30, 3, 6))
     reg = cubeviz_helper.app.get_subsets("Subset 1")
     ellipse1 = EllipsePixelRegion(center=PixCoord(x=30, y=30),
-                                  width=3, height=6, angle=0.0 * u.deg)
+                                  width=6, height=12, angle=0.0 * u.deg)
     assert reg[-1] == {'name': 'EllipticalROI', 'glue_state': 'OrState', 'region': ellipse1,
                        'subset_state': reg[-1]['subset_state']}
 
@@ -384,18 +384,18 @@ def test_composite_region_with_consecutive_and_not_states(cubeviz_helper):
     viewer.apply_roi(EllipticalROI(30, 30, 3, 6))
     reg = cubeviz_helper.app.get_subsets("Subset 1")
     ellipse1 = EllipsePixelRegion(center=PixCoord(x=30, y=30),
-                                  width=3, height=6, angle=0.0 * u.deg)
+                                  width=6, height=12, angle=0.0 * u.deg)
     assert reg[-1] == {'name': 'EllipticalROI', 'glue_state': 'AndNotState', 'region': ellipse1,
                        'subset_state': reg[-1]['subset_state']}
 
     regions_list = cubeviz_helper.app.get_subsets("Subset 1", object_only=True)
     assert len(regions_list) == 3
-    assert regions_list[-1].width == 3
+    assert regions_list[-1].width == 6
 
     regions_list = cubeviz_helper.app.get_subsets("Subset 1", spatial_only=True,
                                                   object_only=True)
     assert len(regions_list) == 3
-    assert regions_list[-1].width == 3
+    assert regions_list[-1].width == 6
 
     spatial_list = cubeviz_helper.app.get_subsets("Subset 1", spatial_only=True)
     assert len(spatial_list) == 3
@@ -446,7 +446,7 @@ def test_composite_region_with_imviz(imviz_helper, image_2d_wcs):
     viewer.apply_roi(EllipticalROI(3, 3, 3, 6))
     reg = imviz_helper.app.get_subsets("Subset 1")
     ellipse1 = EllipsePixelRegion(center=PixCoord(x=3, y=3),
-                                  width=3, height=6, angle=0.0 * u.deg)
+                                  width=6, height=12, angle=0.0 * u.deg)
     assert reg[-1] == {'name': 'EllipticalROI', 'glue_state': 'AndNotState', 'region': ellipse1,
                        'subset_state': reg[-1]['subset_state']}