From d5bdeaf0f658c95a9dd82bb35d9991b7816d9366 Mon Sep 17 00:00:00 2001
From: "Pey Lian Lim (Github)" <2090236+pllim@users.noreply.github.com>
Date: Thu, 3 Nov 2022 21:00:59 -0400
Subject: [PATCH 1/8] POC: Programmatically edit subsets
---
.../imviz_edit_subset_programmatic.ipynb | 158 ++++++++++++++++++
1 file changed, 158 insertions(+)
create mode 100644 notebooks/concepts/imviz_edit_subset_programmatic.ipynb
diff --git a/notebooks/concepts/imviz_edit_subset_programmatic.ipynb b/notebooks/concepts/imviz_edit_subset_programmatic.ipynb
new file mode 100644
index 0000000000..ab19dee50c
--- /dev/null
+++ b/notebooks/concepts/imviz_edit_subset_programmatic.ipynb
@@ -0,0 +1,158 @@
+{
+ "cells": [
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "id": "2dd982a0",
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "import time\n",
+ "\n",
+ "import numpy as np\n",
+ "from glue.core.edit_subset_mode import ReplaceMode\n",
+ "from regions import PixCoord, CirclePixelRegion, RectanglePixelRegion\n",
+ "\n",
+ "from jdaviz import Imviz"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "id": "c6734f52",
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "arr = np.zeros((100, 100))"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "id": "f0906159",
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "rect = RectanglePixelRegion(center=PixCoord(x=40, y=20), width=30, height=5)\n",
+ "circ = CirclePixelRegion(center=PixCoord(x=20, y=80), radius=5)"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "id": "0b54561e",
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "imviz = Imviz()\n",
+ "imviz.load_data(arr, data_label='Pong')"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "id": "635d3952-e5c9-4428-91b0-d86c41d535d3",
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "imviz.show()"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "id": "2030727b",
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "imviz.load_regions([rect, circ])"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "id": "08eab464",
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "imviz.default_viewer.zoom_level = 5"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "id": "bfaa6bb9",
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "subset_tool = imviz.plugins['Subset Tools']"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "id": "594c4fa8",
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "subset_tool.open_in_tray()"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "id": "46b47696",
+ "metadata": {},
+ "outputs": [],
+ "source": [
+ "for i in range(10):\n",
+ " subset_tool._obj.subset_selected = 'Subset 1'\n",
+ " subset_state = subset_tool._obj.subset_select.selected_subset_state\n",
+ " subset_roi = subset_state.roi\n",
+ " cur_x, cur_y = subset_roi.center() # API in glue is inconsistent for this; shape-dependent\n",
+ " subset_roi.move_to(cur_x + 5, cur_y)\n",
+ " subset_tool._obj.session.edit_subset_mode._combine_data(subset_state, override_mode=ReplaceMode)\n",
+ " \n",
+ " time.sleep(0.5)\n",
+ " \n",
+ " subset_tool._obj.subset_selected = 'Subset 2'\n",
+ " subset_state = subset_tool._obj.subset_select.selected_subset_state\n",
+ " subset_roi = subset_state.roi\n",
+ " cur_x, cur_y = subset_roi.get_center() # Circle API is different from rectangle.\n",
+ " subset_roi.move_to(2, -2) # For circle, this is delta, not absolute.\n",
+ " subset_tool._obj.session.edit_subset_mode._combine_data(subset_state, override_mode=ReplaceMode)\n",
+ " \n",
+ " time.sleep(0.5)"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": null,
+ "id": "fe2c1fd3",
+ "metadata": {},
+ "outputs": [],
+ "source": []
+ }
+ ],
+ "metadata": {
+ "kernelspec": {
+ "display_name": "Python 3 (ipykernel)",
+ "language": "python",
+ "name": "python3"
+ },
+ "language_info": {
+ "codemirror_mode": {
+ "name": "ipython",
+ "version": 3
+ },
+ "file_extension": ".py",
+ "mimetype": "text/x-python",
+ "name": "python",
+ "nbconvert_exporter": "python",
+ "pygments_lexer": "ipython3",
+ "version": "3.10.4"
+ }
+ },
+ "nbformat": 4,
+ "nbformat_minor": 5
+}
From 4ea27bcb9a076ad76233949c9c714da6f6dd801e Mon Sep 17 00:00:00 2001
From: "Pey Lian Lim (Github)" <2090236+pllim@users.noreply.github.com>
Date: Mon, 7 Nov 2022 17:19:19 -0500
Subject: [PATCH 2/8] Nicer API for centering
---
CHANGES.rst | 2 +
.../plugins/subset_plugin/subset_plugin.py | 41 +++++++++++++++++++
.../imviz_edit_subset_programmatic.ipynb | 25 +++++------
3 files changed, 53 insertions(+), 15 deletions(-)
diff --git a/CHANGES.rst b/CHANGES.rst
index b7b79c30d0..0ef865de0b 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -8,6 +8,8 @@ New Features
- Spinner in plot options while processing changes to contour settings. [#1794]
+- Subset Tools plugin now allows recentering of spatial subset. [#1823]
+
Cubeviz
^^^^^^^
diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
index faf3fbaffe..64328742ef 100644
--- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
+++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
@@ -226,6 +226,47 @@ def vue_update_subset(self, *args):
self.hub.broadcast(SnackbarMessage(
f"Failed to update Subset: {repr(err)}", color='error', sender=self))
+ def get_center(self):
+ # Composite region cannot be edited, so just grab first element.
+ if not self.is_editable or self.subset_types[0] == "Range": # no-op
+ return
+
+ subset_state = self.subset_select.selected_subset_state
+ sbst_obj = subset_state.roi
+
+ if isinstance(sbst_obj, (CircularROI, EllipticalROI)):
+ cx, cy = sbst_obj.get_center()
+ elif isinstance(sbst_obj, RectangularROI):
+ cx, cy = sbst_obj.center()
+ else: # pragma: no cover
+ raise NotImplementedError(f'Getting center of {sbst_obj.__class__} is not supported')
+
+ return cx, cy
+
+ def set_center(self, x, y):
+ # Composite region cannot be edited, so just grab first element.
+ if not self.is_editable or self.subset_types[0] == "Range": # no-op
+ return
+
+ subset_state = self.subset_select.selected_subset_state
+ sbst_obj = subset_state.roi
+
+ if isinstance(sbst_obj, (CircularROI, EllipticalROI)):
+ self._set_value_in_subset_definition(0, "X Center", "value", x)
+ self._set_value_in_subset_definition(0, "Y Center", "value", y)
+ elif isinstance(sbst_obj, RectangularROI):
+ cx, cy = sbst_obj.center()
+ dx = x - cx
+ dy = y - cy
+ self._set_value_in_subset_definition(0, "Xmin", "value", sbst_obj.xmin + dx)
+ self._set_value_in_subset_definition(0, "Xmax", "value", sbst_obj.xmax + dx)
+ self._set_value_in_subset_definition(0, "Ymin", "value", sbst_obj.ymin + dy)
+ self._set_value_in_subset_definition(0, "Ymax", "value", sbst_obj.ymax + dy)
+ else: # pragma: no cover
+ raise NotImplementedError(f'Recentering of {sbst_obj.__class__} is not supported')
+
+ self.vue_update_subset()
+
# List of JSON-like dict is nice for front-end but a pain to look up,
# so we use these helper functions.
diff --git a/notebooks/concepts/imviz_edit_subset_programmatic.ipynb b/notebooks/concepts/imviz_edit_subset_programmatic.ipynb
index ab19dee50c..db4eae4ca6 100644
--- a/notebooks/concepts/imviz_edit_subset_programmatic.ipynb
+++ b/notebooks/concepts/imviz_edit_subset_programmatic.ipynb
@@ -10,7 +10,6 @@
"import time\n",
"\n",
"import numpy as np\n",
- "from glue.core.edit_subset_mode import ReplaceMode\n",
"from regions import PixCoord, CirclePixelRegion, RectanglePixelRegion\n",
"\n",
"from jdaviz import Imviz"
@@ -105,23 +104,19 @@
"metadata": {},
"outputs": [],
"source": [
+ "plg = subset_tool._obj\n",
+ "\n",
"for i in range(10):\n",
- " subset_tool._obj.subset_selected = 'Subset 1'\n",
- " subset_state = subset_tool._obj.subset_select.selected_subset_state\n",
- " subset_roi = subset_state.roi\n",
- " cur_x, cur_y = subset_roi.center() # API in glue is inconsistent for this; shape-dependent\n",
- " subset_roi.move_to(cur_x + 5, cur_y)\n",
- " subset_tool._obj.session.edit_subset_mode._combine_data(subset_state, override_mode=ReplaceMode)\n",
- " \n",
+ " plg.subset_selected = 'Subset 1'\n",
+ " cur_x, cur_y = plg.get_center()\n",
+ " plg.set_center(cur_x + 5, cur_y)\n",
+ "\n",
" time.sleep(0.5)\n",
" \n",
- " subset_tool._obj.subset_selected = 'Subset 2'\n",
- " subset_state = subset_tool._obj.subset_select.selected_subset_state\n",
- " subset_roi = subset_state.roi\n",
- " cur_x, cur_y = subset_roi.get_center() # Circle API is different from rectangle.\n",
- " subset_roi.move_to(2, -2) # For circle, this is delta, not absolute.\n",
- " subset_tool._obj.session.edit_subset_mode._combine_data(subset_state, override_mode=ReplaceMode)\n",
- " \n",
+ " plg.subset_selected = 'Subset 2'\n",
+ " cur_x, cur_y = plg.get_center()\n",
+ " plg.set_center(cur_x + 2, cur_y - 2)\n",
+ "\n",
" time.sleep(0.5)"
]
},
From ac4e8282ea5116c99c48afb18f8ff608eeffae0f Mon Sep 17 00:00:00 2001
From: "Pey Lian Lim (Github)" <2090236+pllim@users.noreply.github.com>
Date: Tue, 8 Nov 2022 18:49:39 -0500
Subject: [PATCH 3/8] WIP: Sub-plugin to recenter subset for Imviz only.
Fix GUI update and dithered use case.
---
CHANGES.rst | 4 +-
docs/imviz/plugins.rst | 11 +-
.../plugins/subset_plugin/subset_plugin.py | 105 +++++++++++++-----
.../plugins/subset_plugin/subset_plugin.vue | 23 ++++
.../aper_phot_simple/aper_phot_simple.py | 28 +----
jdaviz/configs/imviz/plugins/viewers.py | 13 ++-
jdaviz/core/region_translators.py | 55 +++++++++
.../imviz_edit_subset_programmatic.ipynb | 13 ++-
8 files changed, 191 insertions(+), 61 deletions(-)
diff --git a/CHANGES.rst b/CHANGES.rst
index 0ef865de0b..e8798fc96d 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -8,8 +8,6 @@ New Features
- Spinner in plot options while processing changes to contour settings. [#1794]
-- Subset Tools plugin now allows recentering of spatial subset. [#1823]
-
Cubeviz
^^^^^^^
@@ -20,6 +18,8 @@ Imviz
- Warnings in aperture photometry plugin when using raw profile with large subsets. [#1801]
+- Subset Tools plugin now allows recentering of editable spatial subset. [#1823]
+
Mosviz
^^^^^^
diff --git a/docs/imviz/plugins.rst b/docs/imviz/plugins.rst
index eca87e3c0b..339ecaef7a 100644
--- a/docs/imviz/plugins.rst
+++ b/docs/imviz/plugins.rst
@@ -44,7 +44,7 @@ This plugin allows you to select an existing subset to modify, or to select
:guilabel:`Create new` to create a new subset by selecting and using the region selector
in the spectrum viewer toolbar. You can also choose the operation that will be
applied by the selector tool. Note that these are synched with the subset tools
-in the app-level toolbar. It does not show static regions loaded
+in the app-level toolbar. It might not show some static regions loaded
via the API unless an interactive region is drawn after.
If an existing subset is selected, the parameters of the subset will also be
@@ -52,6 +52,15 @@ shown. Note that while parameters for compound regions (e.g., a subset with
multiple disjoint regions) are displayed, the logical operations joining them
(``OR``, ``AND``, etc.) are not shown.
+For a simple subset in Imviz only, you can choose to recenter it based
+on the selected Data. The centroid is calculated by
+:attr:`photutils.aperture.ApertureStats.centroid`, which is the
+center-of-mass of the data within the aperture.
+No background subtraction is performed. Click :guilabel:`Recenter`
+to change its parameters to the calculated centroid. However, the subset
+will not be moved to the new center until :guilabel:`Update` is also clicked
+(see below).
+
For a simple subset, you can edit its parameters by changing the values
in the corresponding editable text fields. Once you have entered the new
value(s), click :guilabel:`Update` to apply. You should see the subset
diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
index 64328742ef..95a3784597 100644
--- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
+++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
@@ -9,7 +9,7 @@
from jdaviz.core.events import SnackbarMessage
from jdaviz.core.registries import tray_registry
-from jdaviz.core.template_mixin import PluginTemplateMixin, SubsetSelect
+from jdaviz.core.template_mixin import PluginTemplateMixin, DatasetSelectMixin, SubsetSelect
__all__ = ['SubsetPlugin']
@@ -23,7 +23,7 @@
@tray_registry('g-subset-plugin', label="Subset Tools")
-class SubsetPlugin(PluginTemplateMixin):
+class SubsetPlugin(PluginTemplateMixin, DatasetSelectMixin):
template_file = __file__, "subset_plugin.vue"
select = List([]).tag(sync=True)
subset_items = List([]).tag(sync=True)
@@ -226,46 +226,95 @@ def vue_update_subset(self, *args):
self.hub.broadcast(SnackbarMessage(
f"Failed to update Subset: {repr(err)}", color='error', sender=self))
+ def vue_recenter_subset(self, *args):
+ # Composite region cannot be edited. This only works for Imviz.
+ if not self.is_editable or self.config != 'imviz': # no-op
+ return
+
+ from photutils.aperture import ApertureStats
+ from jdaviz.core.region_translators import regions2aperture, _get_region_from_spatial_subset
+
+ reg = _get_region_from_spatial_subset(self, self.subset_selected)
+ aperture = regions2aperture(reg)
+ data = self.dataset.selected_dc_item
+ comp = data.get_component(data.main_components[0])
+ comp_data = comp.data
+ phot_aperstats = ApertureStats(comp_data, aperture)
+
+ # Centroid was calculated in selected data, which might or might not be
+ # the reference data. However, Subset is always defined w.r.t.
+ # the reference data, so we need to convert back.
+ viewer = self.app._jdaviz_helper.default_viewer
+ x, y, _ = viewer._get_real_xy(
+ data, phot_aperstats.xcentroid, phot_aperstats.ycentroid, reverse=True)
+ self.set_center((x, y), update=False)
+
def get_center(self):
- # Composite region cannot be edited, so just grab first element.
- if not self.is_editable or self.subset_types[0] == "Range": # no-op
+ # Composite region cannot be edited.
+ if not self.is_editable: # no-op
return
subset_state = self.subset_select.selected_subset_state
- sbst_obj = subset_state.roi
- if isinstance(sbst_obj, (CircularROI, EllipticalROI)):
- cx, cy = sbst_obj.get_center()
- elif isinstance(sbst_obj, RectangularROI):
- cx, cy = sbst_obj.center()
+ if isinstance(subset_state, RoiSubsetState):
+ sbst_obj = subset_state.roi
+ if isinstance(sbst_obj, (CircularROI, EllipticalROI)):
+ cen = sbst_obj.get_center()
+ elif isinstance(sbst_obj, RectangularROI):
+ cen = sbst_obj.center()
+ else: # pragma: no cover
+ raise NotImplementedError(
+ f'Getting center of {sbst_obj.__class__} is not supported')
+
+ elif isinstance(subset_state, RangeSubsetState):
+ cen = (subset_state.hi - subset_state.lo) * 0.5 + subset_state.lo
+
else: # pragma: no cover
- raise NotImplementedError(f'Getting center of {sbst_obj.__class__} is not supported')
+ raise NotImplementedError(
+ f'Getting center of {subset_state.__class__} is not supported')
- return cx, cy
+ return cen
- def set_center(self, x, y):
+ def set_center(self, new_cen, update=False):
# Composite region cannot be edited, so just grab first element.
- if not self.is_editable or self.subset_types[0] == "Range": # no-op
+ if not self.is_editable: # no-op
return
subset_state = self.subset_select.selected_subset_state
- sbst_obj = subset_state.roi
-
- if isinstance(sbst_obj, (CircularROI, EllipticalROI)):
- self._set_value_in_subset_definition(0, "X Center", "value", x)
- self._set_value_in_subset_definition(0, "Y Center", "value", y)
- elif isinstance(sbst_obj, RectangularROI):
- cx, cy = sbst_obj.center()
- dx = x - cx
- dy = y - cy
- self._set_value_in_subset_definition(0, "Xmin", "value", sbst_obj.xmin + dx)
- self._set_value_in_subset_definition(0, "Xmax", "value", sbst_obj.xmax + dx)
- self._set_value_in_subset_definition(0, "Ymin", "value", sbst_obj.ymin + dy)
- self._set_value_in_subset_definition(0, "Ymax", "value", sbst_obj.ymax + dy)
+
+ if isinstance(subset_state, RoiSubsetState):
+ x, y = new_cen
+ sbst_obj = subset_state.roi
+ if isinstance(sbst_obj, (CircularROI, EllipticalROI)):
+ self._set_value_in_subset_definition(0, "X Center", "value", x)
+ self._set_value_in_subset_definition(0, "Y Center", "value", y)
+ elif isinstance(sbst_obj, RectangularROI):
+ cx, cy = sbst_obj.center()
+ dx = x - cx
+ dy = y - cy
+ self._set_value_in_subset_definition(0, "Xmin", "value", sbst_obj.xmin + dx)
+ self._set_value_in_subset_definition(0, "Xmax", "value", sbst_obj.xmax + dx)
+ self._set_value_in_subset_definition(0, "Ymin", "value", sbst_obj.ymin + dy)
+ self._set_value_in_subset_definition(0, "Ymax", "value", sbst_obj.ymax + dy)
+ else: # pragma: no cover
+ raise NotImplementedError(f'Recentering of {sbst_obj.__class__} is not supported')
+
+ elif isinstance(subset_state, RangeSubsetState):
+ dx = new_cen - ((subset_state.hi - subset_state.lo) * 0.5 + subset_state.lo)
+ self._set_value_in_subset_definition(0, "Lower bound", "value", subset_state.lo + dx)
+ self._set_value_in_subset_definition(0, "Upper bound", "value", subset_state.hi + dx)
+
else: # pragma: no cover
- raise NotImplementedError(f'Recentering of {sbst_obj.__class__} is not supported')
+ raise NotImplementedError(
+ f'Getting center of {subset_state.__class__} is not supported')
- self.vue_update_subset()
+ if update:
+ self.vue_update_subset()
+ else:
+ # Force UI to update on browser without changing the subset.
+ tmp = self.subset_definitions
+ self.subset_definitions = []
+ self.subset_definitions = tmp
# List of JSON-like dict is nice for front-end but a pain to look up,
# so we use these helper functions.
diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue
index de2a1b7725..31faf9dd3e 100644
--- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue
+++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue
@@ -22,6 +22,29 @@
+
+
+
+
+
+ Recenter
+
+
+
+
+ Recenter
+
+
+
+
+
+
Date: Wed, 9 Nov 2022 17:59:08 -0500
Subject: [PATCH 4/8] Added tests
---
.../plugins/subset_plugin/subset_plugin.py | 36 ++++++-----
.../imviz/tests/test_subset_centroid.py | 61 +++++++++++++++++++
jdaviz/tests/test_subsets.py | 30 +++++++++
3 files changed, 113 insertions(+), 14 deletions(-)
create mode 100644 jdaviz/configs/imviz/tests/test_subset_centroid.py
diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
index 95a3784597..af9947304d 100644
--- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
+++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
@@ -234,20 +234,28 @@ def vue_recenter_subset(self, *args):
from photutils.aperture import ApertureStats
from jdaviz.core.region_translators import regions2aperture, _get_region_from_spatial_subset
- reg = _get_region_from_spatial_subset(self, self.subset_selected)
- aperture = regions2aperture(reg)
- data = self.dataset.selected_dc_item
- comp = data.get_component(data.main_components[0])
- comp_data = comp.data
- phot_aperstats = ApertureStats(comp_data, aperture)
-
- # Centroid was calculated in selected data, which might or might not be
- # the reference data. However, Subset is always defined w.r.t.
- # the reference data, so we need to convert back.
- viewer = self.app._jdaviz_helper.default_viewer
- x, y, _ = viewer._get_real_xy(
- data, phot_aperstats.xcentroid, phot_aperstats.ycentroid, reverse=True)
- self.set_center((x, y), update=False)
+ try:
+ reg = _get_region_from_spatial_subset(self, self.subset_selected)
+ aperture = regions2aperture(reg)
+ data = self.dataset.selected_dc_item
+ comp = data.get_component(data.main_components[0])
+ comp_data = comp.data
+ phot_aperstats = ApertureStats(comp_data, aperture)
+
+ # Centroid was calculated in selected data, which might or might not be
+ # the reference data. However, Subset is always defined w.r.t.
+ # the reference data, so we need to convert back.
+ viewer = self.app._jdaviz_helper.default_viewer
+ x, y, _ = viewer._get_real_xy(
+ data, phot_aperstats.xcentroid, phot_aperstats.ycentroid, reverse=True)
+ if not np.all(np.isfinite((x, y))):
+ raise ValueError(f'Invalid centroid ({x}, {y})')
+ except Exception as err:
+ self.set_center(self.get_center(), update=False)
+ self.hub.broadcast(SnackbarMessage(
+ f"Failed to calculate centroid: {repr(err)}", color='error', sender=self))
+ else:
+ self.set_center((x, y), update=False)
def get_center(self):
# Composite region cannot be edited.
diff --git a/jdaviz/configs/imviz/tests/test_subset_centroid.py b/jdaviz/configs/imviz/tests/test_subset_centroid.py
new file mode 100644
index 0000000000..dcb809c3c6
--- /dev/null
+++ b/jdaviz/configs/imviz/tests/test_subset_centroid.py
@@ -0,0 +1,61 @@
+from regions import PixCoord, CirclePixelRegion
+
+from jdaviz.configs.imviz.tests.utils import BaseImviz_WCS_GWCS
+
+
+class TestImvizSpatialSubsetCentroid(BaseImviz_WCS_GWCS):
+ def test_centroiding(self):
+ reg = CirclePixelRegion(PixCoord(2, 2), 3)
+ self.imviz.load_regions(reg)
+
+ plg = self.imviz.plugins['Subset Tools']
+ plg._obj.subset_selected = 'Subset 1'
+
+ # Since they are linked by pixels, the bright corner pixel aligns
+ # and nothing should change.
+ for data_label in ('fits_wcs[DATA]', 'gwcs[DATA]', 'no_wcs'):
+ plg._obj.dataset_selected = data_label
+ plg._obj.vue_recenter_subset()
+
+ # Calculate centroid but do not apply yet.
+ for key in ("X Center", "Y Center"):
+ assert plg._obj._get_value_from_subset_definition(0, key, "value") == -1
+ assert plg._obj._get_value_from_subset_definition(0, key, "orig") == 2
+
+ # Radius will not be touched.
+ for key in ("value", "orig"):
+ assert plg._obj._get_value_from_subset_definition(0, "Radius", key) == 3
+
+ assert plg._obj.get_center() == (2, 2) # Still the original center
+
+ # Now it is at the new centroid.
+ plg._obj.vue_update_subset()
+ assert plg._obj.get_center() == (-1, -1)
+
+ # FITS WCS and GWCS are rotated from each other.
+ # Plain array always by pixel wrt FITS WCS.
+ self.imviz.link_data(link_type='wcs')
+
+ # Move the Subset back first.
+ plg._obj.dataset_selected = 'fits_wcs[DATA]'
+ plg._obj.set_center((2, 2), update=True)
+
+ plg._obj.vue_recenter_subset()
+ for key in ("X Center", "Y Center"):
+ assert plg._obj._get_value_from_subset_definition(0, key, "value") == -1
+ assert plg._obj._get_value_from_subset_definition(0, key, "orig") == 2
+
+ # GWCS does not extrapolate and this Subset is out of bounds,
+ # so will get NaNs and enter the exception handling logic.
+ plg._obj.dataset_selected = 'gwcs[DATA]'
+ plg._obj.vue_recenter_subset()
+ for key in ("X Center", "Y Center"):
+ assert plg._obj._get_value_from_subset_definition(0, key, "value") == 2
+ assert plg._obj._get_value_from_subset_definition(0, key, "orig") == 2
+
+ # No WCS case will be same as FITS WCS.
+ plg._obj.dataset_selected = 'no_wcs'
+ plg._obj.vue_recenter_subset()
+ for key in ("X Center", "Y Center"):
+ assert plg._obj._get_value_from_subset_definition(0, key, "value") == -1
+ assert plg._obj._get_value_from_subset_definition(0, key, "orig") == 2
diff --git a/jdaviz/tests/test_subsets.py b/jdaviz/tests/test_subsets.py
index 70c0a73bbe..2bf151d37f 100644
--- a/jdaviz/tests/test_subsets.py
+++ b/jdaviz/tests/test_subsets.py
@@ -43,6 +43,15 @@ def test_region_from_subset_2d(cubeviz_helper):
assert subset_plugin._get_value_from_subset_definition(0, "Y Radius", key) == 3.3
assert subset_plugin._get_value_from_subset_definition(0, "Angle", key) == 0
+ # Recenter GUI should not be exposed, but API call should also not crash.
+ subset_plugin.vue_recenter_subset()
+ for key in ("orig", "value"):
+ assert subset_plugin._get_value_from_subset_definition(0, "X Center", key) == 1
+ assert subset_plugin._get_value_from_subset_definition(0, "Y Center", key) == 3.5
+ assert subset_plugin._get_value_from_subset_definition(0, "X Radius", key) == 1.2
+ assert subset_plugin._get_value_from_subset_definition(0, "Y Radius", key) == 3.3
+ assert subset_plugin._get_value_from_subset_definition(0, "Angle", key) == 0
+
def test_region_from_subset_3d(cubeviz_helper):
data = Data(flux=np.ones((128, 128, 256)), label='Test 3D Flux')
@@ -70,6 +79,7 @@ def test_region_from_subset_3d(cubeviz_helper):
assert subset_plugin.subset_selected == "Subset 1"
assert subset_plugin.subset_types == ["RectangularROI"]
assert subset_plugin.is_editable
+ assert subset_plugin.get_center() == (2.25, 1.55)
for key in ("orig", "value"):
assert subset_plugin._get_value_from_subset_definition(0, "Xmin", key) == 1
assert subset_plugin._get_value_from_subset_definition(0, "Xmax", key) == 3.5
@@ -102,6 +112,16 @@ def test_region_from_subset_3d(cubeviz_helper):
assert_allclose(reg.height, 3.3)
assert_allclose(reg.angle.to_value(u.deg), 45) # Might be stored in radians
+ # Move the rectangle
+ subset_plugin.set_center((3, 2), update=True)
+ subsets = cubeviz_helper.app.get_subsets_from_viewer('flux-viewer')
+ reg = subsets.get('Subset 1')
+ assert_allclose(reg.center.x, 3)
+ assert_allclose(reg.center.y, 2)
+ assert_allclose(reg.width, 1.5)
+ assert_allclose(reg.height, 3.3)
+ assert_allclose(reg.angle.to_value(u.deg), 45) # Might be stored in radians
+
# Circular Subset
flux_viewer = cubeviz_helper.app.get_viewer("flux-viewer")
# We set the active tool here to trigger a reset of the Subset state to "Create New"
@@ -137,6 +157,7 @@ def test_region_from_subset_profile(cubeviz_helper, spectral_cube_wcs):
assert subset_plugin.subset_selected == "Subset 1"
assert subset_plugin.subset_types == ["Range"]
assert subset_plugin.is_editable
+ assert_allclose(subset_plugin.get_center(), 10.25)
for key in ("orig", "value"):
assert subset_plugin._get_value_from_subset_definition(0, "Lower bound", key) == 5
assert subset_plugin._get_value_from_subset_definition(0, "Upper bound", key) == 15.5
@@ -156,6 +177,13 @@ def test_region_from_subset_profile(cubeviz_helper, spectral_cube_wcs):
assert_quantity_allclose(reg.lower, 10.0 * u.Hz)
assert_quantity_allclose(reg.upper, 15.0 * u.Hz)
+ # Move the Subset.
+ subset_plugin.set_center(10, update=True)
+ subsets = cubeviz_helper.app.get_subsets_from_viewer('spectrum-viewer', subset_type='spectral')
+ reg = subsets.get('Subset 1')
+ assert_quantity_allclose(reg.lower, 8 * u.Hz)
+ assert_quantity_allclose(reg.upper, 12 * u.Hz)
+
def test_region_spectral_spatial(cubeviz_helper, spectral_cube_wcs):
data = Data(flux=np.ones((128, 128, 256)), label='Test Flux', coords=spectral_cube_wcs)
@@ -233,6 +261,8 @@ def test_disjoint_spectral_subset(cubeviz_helper, spectral_cube_wcs):
assert subset_plugin.subset_selected == "Subset 1"
assert subset_plugin.subset_types == ["Range", "Range"]
assert not subset_plugin.is_editable
+ assert subset_plugin.get_center() is None
+ subset_plugin.set_center(99, update=True) # This is no-op
for key in ("orig", "value"):
assert subset_plugin._get_value_from_subset_definition(0, "Lower bound", key) == 30
assert subset_plugin._get_value_from_subset_definition(0, "Upper bound", key) == 35
From f052469c58a3bf9e2a40dbdc8a4ab1ae26e674ea Mon Sep 17 00:00:00 2001
From: Kyle Conroy
Date: Thu, 10 Nov 2022 09:03:04 -0500
Subject: [PATCH 5/8] keep expansion panel opened when redrawn
---
jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py | 4 +++-
.../configs/default/plugins/subset_plugin/subset_plugin.vue | 4 ++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
index af9947304d..473ca1f03e 100644
--- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
+++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
@@ -5,7 +5,7 @@
from glue.core.roi import CircularROI, EllipticalROI, RectangularROI
from glue.core.subset import RoiSubsetState, RangeSubsetState, CompositeSubsetState
from glue_jupyter.widgets.subset_mode_vuetify import SelectionModeMenu
-from traitlets import List, Unicode, Bool, observe
+from traitlets import Integer, List, Unicode, Bool, observe
from jdaviz.core.events import SnackbarMessage
from jdaviz.core.registries import tray_registry
@@ -34,6 +34,8 @@ class SubsetPlugin(PluginTemplateMixin, DatasetSelectMixin):
subset_definitions = List([]).tag(sync=True)
has_subset_details = Bool(False).tag(sync=True)
+ subplugins_opened = Integer().tag(sync=True)
+
is_editable = Bool(False).tag(sync=True)
def __init__(self, *args, **kwargs):
diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue
index 31faf9dd3e..895f798fc1 100644
--- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue
+++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue
@@ -24,9 +24,9 @@
-
+
-
+
Recenter
From fd7f6715bd867281705c716d448a33900cb50690 Mon Sep 17 00:00:00 2001
From: "Pey Lian Lim (Github)" <2090236+pllim@users.noreply.github.com>
Date: Thu, 10 Nov 2022 12:19:19 -0500
Subject: [PATCH 6/8] Address Kyle comments
---
.../plugins/subset_plugin/subset_plugin.py | 39 ++++++++++++++++++-
jdaviz/tests/test_subsets.py | 12 ++----
2 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
index 473ca1f03e..50633b5e20 100644
--- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
+++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
@@ -231,7 +231,8 @@ def vue_update_subset(self, *args):
def vue_recenter_subset(self, *args):
# Composite region cannot be edited. This only works for Imviz.
if not self.is_editable or self.config != 'imviz': # no-op
- return
+ raise NotImplementedError(
+ f'Cannot recenter: is_editable={self.is_editable}, config={self.config}')
from photutils.aperture import ApertureStats
from jdaviz.core.region_translators import regions2aperture, _get_region_from_spatial_subset
@@ -260,6 +261,22 @@ def vue_recenter_subset(self, *args):
self.set_center((x, y), update=False)
def get_center(self):
+ """Return the center of the Subset.
+ This may or may not be the centroid obtain from data.
+
+ Returns
+ -------
+ cen : number, tuple of numbers, or `None`
+ The center of the Subset in ``x`` or ``(x, y)``,
+ depending on the Subset type, if applicable.
+ If Subset is not editable, this returns `None`.
+
+ Raises
+ ------
+ NotImplementedError
+ Subset type is not supported.
+
+ """
# Composite region cannot be edited.
if not self.is_editable: # no-op
return
@@ -286,6 +303,26 @@ def get_center(self):
return cen
def set_center(self, new_cen, update=False):
+ """Set the desired center for the selected Subset, if applicable.
+ If Subset is not editable, nothing is done.
+
+ Parameters
+ ----------
+ new_cen : number or tuple of numbers
+ The new center defined either as ``x`` or ``(x, y)``,
+ depending on the Subset type.
+
+ update : bool
+ If `True`, the Subset is also moved to the new center.
+ Otherwise, only the relevant editable fields are updated but the
+ Subset is not moved.
+
+ Raises
+ ------
+ NotImplementedError
+ Subset type is not supported.
+
+ """
# Composite region cannot be edited, so just grab first element.
if not self.is_editable: # no-op
return
diff --git a/jdaviz/tests/test_subsets.py b/jdaviz/tests/test_subsets.py
index 2bf151d37f..64cc4bf532 100644
--- a/jdaviz/tests/test_subsets.py
+++ b/jdaviz/tests/test_subsets.py
@@ -1,4 +1,5 @@
import numpy as np
+import pytest
from astropy import units as u
from astropy.tests.helper import assert_quantity_allclose
from glue.core import Data
@@ -43,14 +44,9 @@ def test_region_from_subset_2d(cubeviz_helper):
assert subset_plugin._get_value_from_subset_definition(0, "Y Radius", key) == 3.3
assert subset_plugin._get_value_from_subset_definition(0, "Angle", key) == 0
- # Recenter GUI should not be exposed, but API call should also not crash.
- subset_plugin.vue_recenter_subset()
- for key in ("orig", "value"):
- assert subset_plugin._get_value_from_subset_definition(0, "X Center", key) == 1
- assert subset_plugin._get_value_from_subset_definition(0, "Y Center", key) == 3.5
- assert subset_plugin._get_value_from_subset_definition(0, "X Radius", key) == 1.2
- assert subset_plugin._get_value_from_subset_definition(0, "Y Radius", key) == 3.3
- assert subset_plugin._get_value_from_subset_definition(0, "Angle", key) == 0
+ # Recenter GUI should not be exposed, but API call would raise exception.
+ with pytest.raises(NotImplementedError, match='Cannot recenter'):
+ subset_plugin.vue_recenter_subset()
def test_region_from_subset_3d(cubeviz_helper):
From c425f7e206e291bf95a4eaab9f4755c9b3c0f662 Mon Sep 17 00:00:00 2001
From: "Pey Lian Lim (Github)" <2090236+pllim@users.noreply.github.com>
Date: Fri, 11 Nov 2022 14:43:09 -0500
Subject: [PATCH 7/8] Recenter now also move the Subset.
Increase test coverage.
---
docs/imviz/plugins.rst | 4 +--
.../plugins/subset_plugin/subset_plugin.py | 2 +-
.../imviz/tests/test_subset_centroid.py | 25 +++++++++++--------
jdaviz/core/tests/test_region_translators.py | 9 ++++++-
4 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/docs/imviz/plugins.rst b/docs/imviz/plugins.rst
index 339ecaef7a..b02b654624 100644
--- a/docs/imviz/plugins.rst
+++ b/docs/imviz/plugins.rst
@@ -57,9 +57,7 @@ on the selected Data. The centroid is calculated by
:attr:`photutils.aperture.ApertureStats.centroid`, which is the
center-of-mass of the data within the aperture.
No background subtraction is performed. Click :guilabel:`Recenter`
-to change its parameters to the calculated centroid. However, the subset
-will not be moved to the new center until :guilabel:`Update` is also clicked
-(see below).
+to change its parameters and move it to the calculated centroid.
For a simple subset, you can edit its parameters by changing the values
in the corresponding editable text fields. Once you have entered the new
diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
index 50633b5e20..4f84310e58 100644
--- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
+++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
@@ -258,7 +258,7 @@ def vue_recenter_subset(self, *args):
self.hub.broadcast(SnackbarMessage(
f"Failed to calculate centroid: {repr(err)}", color='error', sender=self))
else:
- self.set_center((x, y), update=False)
+ self.set_center((x, y), update=True)
def get_center(self):
"""Return the center of the Subset.
diff --git a/jdaviz/configs/imviz/tests/test_subset_centroid.py b/jdaviz/configs/imviz/tests/test_subset_centroid.py
index dcb809c3c6..4505434e21 100644
--- a/jdaviz/configs/imviz/tests/test_subset_centroid.py
+++ b/jdaviz/configs/imviz/tests/test_subset_centroid.py
@@ -15,39 +15,35 @@ def test_centroiding(self):
# and nothing should change.
for data_label in ('fits_wcs[DATA]', 'gwcs[DATA]', 'no_wcs'):
plg._obj.dataset_selected = data_label
+ plg._obj.set_center((2, 2), update=True) # Move the Subset back first.
plg._obj.vue_recenter_subset()
- # Calculate centroid but do not apply yet.
+ # Calculate and move to centroid.
for key in ("X Center", "Y Center"):
assert plg._obj._get_value_from_subset_definition(0, key, "value") == -1
- assert plg._obj._get_value_from_subset_definition(0, key, "orig") == 2
+ assert plg._obj._get_value_from_subset_definition(0, key, "orig") == -1
# Radius will not be touched.
for key in ("value", "orig"):
assert plg._obj._get_value_from_subset_definition(0, "Radius", key) == 3
- assert plg._obj.get_center() == (2, 2) # Still the original center
-
- # Now it is at the new centroid.
- plg._obj.vue_update_subset()
assert plg._obj.get_center() == (-1, -1)
# FITS WCS and GWCS are rotated from each other.
# Plain array always by pixel wrt FITS WCS.
self.imviz.link_data(link_type='wcs')
- # Move the Subset back first.
plg._obj.dataset_selected = 'fits_wcs[DATA]'
- plg._obj.set_center((2, 2), update=True)
-
+ plg._obj.set_center((2, 2), update=True) # Move the Subset back first.
plg._obj.vue_recenter_subset()
for key in ("X Center", "Y Center"):
assert plg._obj._get_value_from_subset_definition(0, key, "value") == -1
- assert plg._obj._get_value_from_subset_definition(0, key, "orig") == 2
+ assert plg._obj._get_value_from_subset_definition(0, key, "orig") == -1
# GWCS does not extrapolate and this Subset is out of bounds,
# so will get NaNs and enter the exception handling logic.
plg._obj.dataset_selected = 'gwcs[DATA]'
+ plg._obj.set_center((2, 2), update=True) # Move the Subset back first.
plg._obj.vue_recenter_subset()
for key in ("X Center", "Y Center"):
assert plg._obj._get_value_from_subset_definition(0, key, "value") == 2
@@ -58,4 +54,11 @@ def test_centroiding(self):
plg._obj.vue_recenter_subset()
for key in ("X Center", "Y Center"):
assert plg._obj._get_value_from_subset_definition(0, key, "value") == -1
- assert plg._obj._get_value_from_subset_definition(0, key, "orig") == 2
+ assert plg._obj._get_value_from_subset_definition(0, key, "orig") == -1
+
+ # This ends up not getting used in production but just in case we need it
+ # back in the future.
+ plg._obj.set_center((2, 2), update=False)
+ for key in ("X Center", "Y Center"):
+ assert plg._obj._get_value_from_subset_definition(0, key, "value") == 2
+ assert plg._obj._get_value_from_subset_definition(0, key, "orig") == -1
diff --git a/jdaviz/core/tests/test_region_translators.py b/jdaviz/core/tests/test_region_translators.py
index 0e305efef6..dcc614669f 100644
--- a/jdaviz/core/tests/test_region_translators.py
+++ b/jdaviz/core/tests/test_region_translators.py
@@ -14,7 +14,8 @@
CircleAnnulusPixelRegion, EllipseAnnulusPixelRegion,
RectangleAnnulusPixelRegion, PolygonPixelRegion, PixCoord)
-from jdaviz.core.region_translators import regions2roi, regions2aperture, aperture2regions
+from jdaviz.core.region_translators import (
+ regions2roi, regions2aperture, aperture2regions, _get_region_from_spatial_subset)
# TODO: Use proper method from upstream when that is available.
@@ -332,3 +333,9 @@ def test_translation_polygon():
regions2aperture(region_shape)
with pytest.raises(NotImplementedError, match='is not supported'):
regions2roi(region_shape)
+
+
+def test_get_region_invalid(imviz_helper):
+ plugin_obj = imviz_helper.plugins['Subset Tools']
+ with pytest.raises(ValueError, match='not found'):
+ _get_region_from_spatial_subset(plugin_obj._obj, 'Subset 99')
From e7c15a4a1ae062029d21e3bf950750d6d5386d51 Mon Sep 17 00:00:00 2001
From: "P. L. Lim" <2090236+pllim@users.noreply.github.com>
Date: Mon, 14 Nov 2022 12:11:11 -0500
Subject: [PATCH 8/8] Fix subplugin when close
Co-authored-by: Kyle Conroy
---
jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
index 4f84310e58..a0984ab71d 100644
--- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
+++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
@@ -5,7 +5,7 @@
from glue.core.roi import CircularROI, EllipticalROI, RectangularROI
from glue.core.subset import RoiSubsetState, RangeSubsetState, CompositeSubsetState
from glue_jupyter.widgets.subset_mode_vuetify import SelectionModeMenu
-from traitlets import Integer, List, Unicode, Bool, observe
+from traitlets import Any, List, Unicode, Bool, observe
from jdaviz.core.events import SnackbarMessage
from jdaviz.core.registries import tray_registry
@@ -34,7 +34,7 @@ class SubsetPlugin(PluginTemplateMixin, DatasetSelectMixin):
subset_definitions = List([]).tag(sync=True)
has_subset_details = Bool(False).tag(sync=True)
- subplugins_opened = Integer().tag(sync=True)
+ subplugins_opened = Any().tag(sync=True)
is_editable = Bool(False).tag(sync=True)