From ef33d35f28678bb3d5f819825f46243ab8565a76 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 4 Apr 2024 16:20:17 -0400 Subject: [PATCH 1/9] Raise error or snackbar if adding an orientation with no data loaded --- .../imviz/plugins/orientation/orientation.py | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/orientation/orientation.py b/jdaviz/configs/imviz/plugins/orientation/orientation.py index e15fa70164..de247a44ae 100644 --- a/jdaviz/configs/imviz/plugins/orientation/orientation.py +++ b/jdaviz/configs/imviz/plugins/orientation/orientation.py @@ -275,6 +275,9 @@ def vue_reset_astrowidget_markers(self, *args): def _get_wcs_angles(self, first_loaded_image=None): if first_loaded_image is None: first_loaded_image = self.viewer.selected_obj.first_loaded_data + if first_loaded_image is None: + # These won't end up getting used in this case, but need an actual number + return 0,0,0 degn, dege, flip = get_compass_info( first_loaded_image.coords, first_loaded_image.shape )[-3:] @@ -293,7 +296,7 @@ def rotation_angle_deg(self, rotation_angle=None): return 0 * u.deg def add_orientation(self, rotation_angle=None, east_left=None, label=None, - set_on_create=True, wrt_data=None): + set_on_create=True, wrt_data=None, from_ui=False): """ Add new orientation options. @@ -332,6 +335,12 @@ def add_orientation(self, rotation_angle=None, east_left=None, label=None, # default rotation: wrt_data = self.viewer.selected_obj.first_loaded_data if wrt_data is None: # Nothing in viewer + msg = "Viewer must have data loaded to add an orientation." + if from_ui: + self.hub.broadcast(SnackbarMessage(msg, color="error", + timeout=6000, sender=self)) + else: + raise ValueError(msg) return rotation_angle = self.rotation_angle_deg(rotation_angle) @@ -414,7 +423,7 @@ def _on_data_add_to_viewer(self, msg): self.viewer.selected_obj.state.reset_limits() def vue_add_orientation(self, *args, **kwargs): - self.add_orientation(set_on_create=True) + self.add_orientation(set_on_create=True, from_ui=True) @observe('orientation_layer_selected') def _change_reference_data(self, *args, **kwargs): @@ -486,7 +495,8 @@ def _on_viewer_change(self, msg={}): if ref_data.label in self.orientation.choices: self.orientation.selected = ref_data.label - def create_north_up_east_left(self, label="North-up, East-left", set_on_create=False): + def create_north_up_east_left(self, label="North-up, East-left", set_on_create=False, + from_ui=False): """ Set the rotation angle and flip to achieve North up and East left according to the reference image WCS. @@ -494,11 +504,13 @@ def create_north_up_east_left(self, label="North-up, East-left", set_on_create=F if label not in self.orientation.choices: degn = self._get_wcs_angles()[-3] self.add_orientation(rotation_angle=degn, east_left=True, - label=label, set_on_create=set_on_create) + label=label, set_on_create=set_on_create, + from_ui=from_ui) elif set_on_create: self.orientation.selected = label - def create_north_up_east_right(self, label="North-up, East-right", set_on_create=False): + def create_north_up_east_right(self, label="North-up, East-right", set_on_create=False, + from_ui=False): """ Set the rotation angle and flip to achieve North up and East right according to the reference image WCS. @@ -506,15 +518,16 @@ def create_north_up_east_right(self, label="North-up, East-right", set_on_create if label not in self.orientation.choices: degn = self._get_wcs_angles()[-3] self.add_orientation(rotation_angle=180 - degn, east_left=False, - label=label, set_on_create=set_on_create) + label=label, set_on_create=set_on_create, + from_ui=from_ui) elif set_on_create: self.orientation.selected = label def vue_select_north_up_east_left(self, *args, **kwargs): - self.create_north_up_east_left(set_on_create=True) + self.create_north_up_east_left(set_on_create=True, from_ui=True) def vue_select_north_up_east_right(self, *args, **kwargs): - self.create_north_up_east_right(set_on_create=True) + self.create_north_up_east_right(set_on_create=True, from_ui=True) def vue_select_default_orientation(self, *args, **kwargs): self.orientation.selected = base_wcs_layer_label From e378353cdc96423426dd5d92869e578347572db3 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Mon, 8 Apr 2024 09:54:17 -0400 Subject: [PATCH 2/9] Don't update with infinite bounds --- jdaviz/core/freezable_state.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jdaviz/core/freezable_state.py b/jdaviz/core/freezable_state.py index 5f77e59222..3fb1d0a1c0 100644 --- a/jdaviz/core/freezable_state.py +++ b/jdaviz/core/freezable_state.py @@ -201,6 +201,10 @@ def reset_limits(self, *event): x_min, x_max, y_min, y_max = self._get_reset_limits() + # If any bound wasn't set to a real value, don't update + if np.any(~np.isfinite([x_min, x_max, y_min, y_max])): + return + with delay_callback(self, 'x_min', 'x_max', 'y_min', 'y_max'): self.x_min, self.x_max, self.y_min, self.y_max = x_min, x_max, y_min, y_max # We need to adjust the limits in here to avoid triggering all From 4a8f83bcf8411a69996da1cc382e91650c1697ff Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Mon, 8 Apr 2024 09:56:41 -0400 Subject: [PATCH 3/9] Codestyle --- jdaviz/configs/imviz/plugins/orientation/orientation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/orientation/orientation.py b/jdaviz/configs/imviz/plugins/orientation/orientation.py index de247a44ae..cebf6b4e8c 100644 --- a/jdaviz/configs/imviz/plugins/orientation/orientation.py +++ b/jdaviz/configs/imviz/plugins/orientation/orientation.py @@ -276,8 +276,8 @@ def _get_wcs_angles(self, first_loaded_image=None): if first_loaded_image is None: first_loaded_image = self.viewer.selected_obj.first_loaded_data if first_loaded_image is None: - # These won't end up getting used in this case, but need an actual number - return 0,0,0 + # These won't end up getting used in this case, but we need an actual number + return 0, 0, 0 degn, dege, flip = get_compass_info( first_loaded_image.coords, first_loaded_image.shape )[-3:] From f7b88324bb836f9b18d107eaa0318bf820a1377b Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Mon, 8 Apr 2024 10:00:20 -0400 Subject: [PATCH 4/9] Add changelog --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index aca4d263ae..b345272b7b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -70,6 +70,8 @@ Cubeviz Imviz ^^^^^ +- Improved behavior when orientations are created or selected without having data loaded in the viewer. [#2789] + Mosviz ^^^^^^ From 8fd66d5cb45ab5fac5900924e1d4af07867c010a Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Mon, 8 Apr 2024 14:58:17 -0400 Subject: [PATCH 5/9] Adding tests --- .../configs/imviz/tests/test_orientation.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/jdaviz/configs/imviz/tests/test_orientation.py b/jdaviz/configs/imviz/tests/test_orientation.py index bbeceeaa0b..d1eebc9222 100644 --- a/jdaviz/configs/imviz/tests/test_orientation.py +++ b/jdaviz/configs/imviz/tests/test_orientation.py @@ -223,3 +223,25 @@ def test_delete_orientation_with_subset(self, klass, angle, sbst_theta): # FIXME: However, sky angle has to stay the same as per regions convention. with pytest.raises(AssertionError, match="Not equal to tolerance"): assert_quantity_allclose(out_reg.angle, reg.angle) + +class TestOrientationNoData(BaseImviz_WCS_WCS): + def test_create_no_data(self): + lc_plugin = self.imviz.plugins['Orientation'] + lc_plugin.link_type = 'WCS' + + viewer_2 = self.imviz.create_image_viewer() + lc_plugin.viewer = "imviz-1" + + with pytest.raises(ValueError, match="Viewer must have data loaded"): + lc_plugin._obj.create_north_up_east_left(set_on_create=True) + + def test_select_no_data(self): + lc_plugin = self.imviz.plugins['Orientation'] + lc_plugin.link_type = 'WCS' + + lc_plugin._obj.create_north_up_east_left(set_on_create=True) + + viewer_2 = self.imviz.create_image_viewer() + lc_plugin.viewer = "imviz-1" + # This would error prior to bugfix + lc_plugin.orientation = "North-up, East-left" From 93ba84a45df259c1e59e98101a13b07f1521e88c Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Mon, 8 Apr 2024 15:04:32 -0400 Subject: [PATCH 6/9] Load data after orientation switch --- jdaviz/configs/imviz/tests/test_orientation.py | 1 + 1 file changed, 1 insertion(+) diff --git a/jdaviz/configs/imviz/tests/test_orientation.py b/jdaviz/configs/imviz/tests/test_orientation.py index d1eebc9222..c977b2f688 100644 --- a/jdaviz/configs/imviz/tests/test_orientation.py +++ b/jdaviz/configs/imviz/tests/test_orientation.py @@ -245,3 +245,4 @@ def test_select_no_data(self): lc_plugin.viewer = "imviz-1" # This would error prior to bugfix lc_plugin.orientation = "North-up, East-left" + self.imviz.app.add_data_to_viewer("imviz-1", "has_wcs_1[SCI,1]") From 5e0015e1937b2e5fe62313c515cec66a602ee3ab Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Mon, 8 Apr 2024 15:23:56 -0400 Subject: [PATCH 7/9] Codestyle --- jdaviz/configs/imviz/tests/test_orientation.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/jdaviz/configs/imviz/tests/test_orientation.py b/jdaviz/configs/imviz/tests/test_orientation.py index c977b2f688..ed2f83b7b2 100644 --- a/jdaviz/configs/imviz/tests/test_orientation.py +++ b/jdaviz/configs/imviz/tests/test_orientation.py @@ -224,12 +224,13 @@ def test_delete_orientation_with_subset(self, klass, angle, sbst_theta): with pytest.raises(AssertionError, match="Not equal to tolerance"): assert_quantity_allclose(out_reg.angle, reg.angle) + class TestOrientationNoData(BaseImviz_WCS_WCS): def test_create_no_data(self): lc_plugin = self.imviz.plugins['Orientation'] lc_plugin.link_type = 'WCS' - viewer_2 = self.imviz.create_image_viewer() + self.imviz.create_image_viewer() lc_plugin.viewer = "imviz-1" with pytest.raises(ValueError, match="Viewer must have data loaded"): @@ -241,7 +242,7 @@ def test_select_no_data(self): lc_plugin._obj.create_north_up_east_left(set_on_create=True) - viewer_2 = self.imviz.create_image_viewer() + self.imviz.create_image_viewer() lc_plugin.viewer = "imviz-1" # This would error prior to bugfix lc_plugin.orientation = "North-up, East-left" From b9227925f47c9631375ad82a5f5ef29bd08b2b5d Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Wed, 10 Apr 2024 10:26:18 -0400 Subject: [PATCH 8/9] Move new keyword to private method --- .../imviz/plugins/orientation/orientation.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/orientation/orientation.py b/jdaviz/configs/imviz/plugins/orientation/orientation.py index cebf6b4e8c..4b695f18ed 100644 --- a/jdaviz/configs/imviz/plugins/orientation/orientation.py +++ b/jdaviz/configs/imviz/plugins/orientation/orientation.py @@ -296,7 +296,7 @@ def rotation_angle_deg(self, rotation_angle=None): return 0 * u.deg def add_orientation(self, rotation_angle=None, east_left=None, label=None, - set_on_create=True, wrt_data=None, from_ui=False): + set_on_create=True, wrt_data=None): """ Add new orientation options. @@ -327,6 +327,12 @@ def add_orientation(self, rotation_angle=None, east_left=None, label=None, nothing will be done. """ + self._add_orientation(rotation_angle=rotation_angle, east_left=east_left, label=label, + set_on_create=set_on_create, wrt_data=wrt_data) + + def _add_orientation(self, rotation_angle=None, east_left=None, label=None, + set_on_create=True, wrt_data=None, from_ui=False): + if self.link_type_selected != 'WCS': raise ValueError("must be aligned by WCS to add orientation options") @@ -423,7 +429,7 @@ def _on_data_add_to_viewer(self, msg): self.viewer.selected_obj.state.reset_limits() def vue_add_orientation(self, *args, **kwargs): - self.add_orientation(set_on_create=True, from_ui=True) + self._add_orientation(set_on_create=True, from_ui=True) @observe('orientation_layer_selected') def _change_reference_data(self, *args, **kwargs): @@ -503,7 +509,7 @@ def create_north_up_east_left(self, label="North-up, East-left", set_on_create=F """ if label not in self.orientation.choices: degn = self._get_wcs_angles()[-3] - self.add_orientation(rotation_angle=degn, east_left=True, + self._add_orientation(rotation_angle=degn, east_left=True, label=label, set_on_create=set_on_create, from_ui=from_ui) elif set_on_create: @@ -517,7 +523,7 @@ def create_north_up_east_right(self, label="North-up, East-right", set_on_create """ if label not in self.orientation.choices: degn = self._get_wcs_angles()[-3] - self.add_orientation(rotation_angle=180 - degn, east_left=False, + self._add_orientation(rotation_angle=180 - degn, east_left=False, label=label, set_on_create=set_on_create, from_ui=from_ui) elif set_on_create: From 68684c59b3f3225ea113cd696dfb09d923e852e7 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Wed, 10 Apr 2024 16:15:15 -0400 Subject: [PATCH 9/9] Fix indents --- jdaviz/configs/imviz/plugins/orientation/orientation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/orientation/orientation.py b/jdaviz/configs/imviz/plugins/orientation/orientation.py index 4b695f18ed..2c50f47e75 100644 --- a/jdaviz/configs/imviz/plugins/orientation/orientation.py +++ b/jdaviz/configs/imviz/plugins/orientation/orientation.py @@ -510,8 +510,8 @@ def create_north_up_east_left(self, label="North-up, East-left", set_on_create=F if label not in self.orientation.choices: degn = self._get_wcs_angles()[-3] self._add_orientation(rotation_angle=degn, east_left=True, - label=label, set_on_create=set_on_create, - from_ui=from_ui) + label=label, set_on_create=set_on_create, + from_ui=from_ui) elif set_on_create: self.orientation.selected = label @@ -524,8 +524,8 @@ def create_north_up_east_right(self, label="North-up, East-right", set_on_create if label not in self.orientation.choices: degn = self._get_wcs_angles()[-3] self._add_orientation(rotation_angle=180 - degn, east_left=False, - label=label, set_on_create=set_on_create, - from_ui=from_ui) + label=label, set_on_create=set_on_create, + from_ui=from_ui) elif set_on_create: self.orientation.selected = label