From deef7493dccca3e0c5464acb0ad13937511dab80 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <39831871+rosteen@users.noreply.github.com> Date: Thu, 11 Apr 2024 15:34:51 -0400 Subject: [PATCH] Fix Orientation selection/creation with no loaded data (#2789) * Raise error or snackbar if adding an orientation with no data loaded * Don't update with infinite bounds * Codestyle * Add changelog * Adding tests * Load data after orientation switch * Codestyle * Move new keyword to private method * Fix indents --- CHANGES.rst | 2 + .../imviz/plugins/orientation/orientation.py | 37 ++++++++++++++----- .../configs/imviz/tests/test_orientation.py | 24 ++++++++++++ jdaviz/core/freezable_state.py | 4 ++ 4 files changed, 58 insertions(+), 9 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 1b19ef5413..60fc7cfd31 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -73,6 +73,8 @@ Imviz ^^^^^ - Fix a bug where footprints did not overlay when created via API. [#2790] +- Improved behavior when orientations are created or selected without having data loaded in the viewer. [#2789] + Mosviz ^^^^^^ diff --git a/jdaviz/configs/imviz/plugins/orientation/orientation.py b/jdaviz/configs/imviz/plugins/orientation/orientation.py index e15fa70164..2c50f47e75 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 we need an actual number + return 0, 0, 0 degn, dege, flip = get_compass_info( first_loaded_image.coords, first_loaded_image.shape )[-3:] @@ -324,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") @@ -332,6 +341,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 +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) + self._add_orientation(set_on_create=True, from_ui=True) @observe('orientation_layer_selected') def _change_reference_data(self, *args, **kwargs): @@ -486,35 +501,39 @@ 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. """ 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) + 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: 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. """ 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) + 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: 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 diff --git a/jdaviz/configs/imviz/tests/test_orientation.py b/jdaviz/configs/imviz/tests/test_orientation.py index bbeceeaa0b..ed2f83b7b2 100644 --- a/jdaviz/configs/imviz/tests/test_orientation.py +++ b/jdaviz/configs/imviz/tests/test_orientation.py @@ -223,3 +223,27 @@ 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' + + 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) + + self.imviz.create_image_viewer() + 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]") 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