Skip to content

Commit

Permalink
Fix Orientation selection/creation with no loaded data (#2789)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
rosteen authored Apr 11, 2024
1 parent fd902ff commit deef749
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 9 deletions.
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^

Expand Down
37 changes: 28 additions & 9 deletions jdaviz/configs/imviz/plugins/orientation/orientation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:]
Expand Down Expand Up @@ -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")

Expand All @@ -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)
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions jdaviz/configs/imviz/tests/test_orientation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]")
4 changes: 4 additions & 0 deletions jdaviz/core/freezable_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit deef749

Please sign in to comment.