Skip to content

Commit

Permalink
Update orientation API to match UI (#3128)
Browse files Browse the repository at this point in the history
* Replace wcs_use_affine with wcs_fast_approximation

* Changelog and codestyle

* Add deprecation warnings

* Update concept notebooks

* Use Astropy deprecation instead

* Fix failing test

* Make deprecation ignores explicit
  • Loading branch information
rosteen authored Aug 5, 2024
1 parent 45734f6 commit f4a6889
Show file tree
Hide file tree
Showing 33 changed files with 220 additions and 187 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ Imviz

- Added Gaia catalog to Catalog plugin. [#3090]

- Updated ``link_type`` to ``align_by`` and ``wcs_use_affine`` to ``wcs_fast_approximation`` in
Orientation plugin API to better match UI text. [#3128]

Mosviz
^^^^^^

Expand Down
12 changes: 6 additions & 6 deletions jdaviz/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,9 +325,9 @@ def __init__(self, configuration=None, *args, **kwargs):
self.auto_link = kwargs.pop('auto_link', True)

# Imviz linking
self._link_type = 'pixels'
self._align_by = 'pixels'
if self.config == "imviz":
self._wcs_use_affine = None
self._wcs_fast_approximation = None

# Subscribe to messages indicating that a new viewer needs to be
# created. When received, information is passed to the application
Expand Down Expand Up @@ -1938,7 +1938,7 @@ def _reparent_subsets(self, old_parent, new_parent=None):

# Translate bounds through WCS if needed
if (self.config == "imviz" and
self._jdaviz_helper.plugins["Orientation"].link_type == "WCS"):
self._jdaviz_helper.plugins["Orientation"].align_by == "WCS"):

# Default shape for WCS-only layers is 10x10, but it doesn't really matter
# since we only need the angles.
Expand Down Expand Up @@ -2219,7 +2219,7 @@ def vue_data_item_remove(self, event):

# Make sure the data isn't loaded in any viewers and isn't the selected orientation
for viewer_id, viewer in self._viewer_store.items():
if orientation_plugin is not None and self._link_type == 'wcs':
if orientation_plugin is not None and self._align_by == 'wcs':
if viewer.state.reference_data.label == data_label:
self._change_reference_data(base_wcs_layer_label, viewer_id)
self.remove_data_from_viewer(viewer_id, data_label)
Expand Down Expand Up @@ -2506,12 +2506,12 @@ def _on_new_viewer(self, msg, vid=None, name=None, add_layers_to_viewer=False,
msg.cls, data=msg.data, show=False)
viewer.figure_widget.layout.height = '100%'

linked_by_wcs = self._link_type == 'wcs'
linked_by_wcs = self._align_by == 'wcs'

if hasattr(viewer.state, 'linked_by_wcs'):
orientation_plugin = self._jdaviz_helper.plugins.get('Orientation', None)
if orientation_plugin is not None:
linked_by_wcs = orientation_plugin.link_type.selected == 'WCS'
linked_by_wcs = orientation_plugin.align_by.selected == 'WCS'
elif len(self._viewer_store) and hasattr(self._jdaviz_helper, 'default_viewer'):
# The plugin would only not exist for instances of Imviz where the user has
# intentionally removed the Orientation plugin, but in that case we will
Expand Down
6 changes: 3 additions & 3 deletions jdaviz/configs/default/plugins/export/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -728,12 +728,12 @@ def save_subset_as_region(self, selected_subset_label, filename):
"""

# type of region saved depends on link type
link_type = getattr(self.app, '_link_type', None)
align_by = getattr(self.app, '_align_by', None)

region = self.app.get_subsets(subset_name=selected_subset_label,
include_sky_region=link_type == 'wcs')
include_sky_region=align_by == 'wcs')

region = region[0][f'{"sky_" if link_type == "wcs" else ""}region']
region = region[0][f'{"sky_" if align_by == "wcs" else ""}region']

region.write(str(filename), overwrite=True)

Expand Down
2 changes: 1 addition & 1 deletion jdaviz/configs/default/plugins/export/tests/test_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def test_export_subsets_wcs(self, imviz_helper, spectral_cube_wcs):
imviz_helper.load_data(data) # load data twice so we can link them
imviz_helper.load_data(data)

imviz_helper.link_data(link_type='wcs')
imviz_helper.link_data(align_by='wcs')

imviz_helper.app.get_viewer('imviz-0').apply_roi(CircularROI(xc=8,
yc=6,
Expand Down
6 changes: 3 additions & 3 deletions jdaviz/configs/default/plugins/markers/markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def _recompute_mark_positions(self, viewer):
orig_world_x = np.asarray(self.table._qtable['world_ra'][in_viewer])
orig_world_y = np.asarray(self.table._qtable['world_dec'][in_viewer])

if self.app._link_type.lower() == 'wcs':
if self.app._align_by.lower() == 'wcs':
# convert from the sky coordinates in the table to pixels via the WCS of the current
# reference data
new_wcs = viewer.state.reference_data.coords
Expand All @@ -139,7 +139,7 @@ def _recompute_mark_positions(self, viewer):
except Exception:
# fail gracefully
new_x, new_y = [], []
elif self.app._link_type == 'pixels':
elif self.app._align_by == 'pixels':
# we need to convert based on the WCS of the individual data layers on which each mark
# was first created
new_x, new_y = np.zeros_like(orig_world_x), np.zeros_like(orig_world_y)
Expand All @@ -156,7 +156,7 @@ def _recompute_mark_positions(self, viewer):
new_x, new_y = [], []
break
else:
raise NotImplementedError(f"link_type {self.app._link_type} not implemented")
raise NotImplementedError(f"align_by {self.app._align_by} not implemented")

# check for entries that do not correspond to a layer or only have pixel coordinates
pixel_only_inds = data_labels == ''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ def _on_global_display_unit_changed(self, *args):
self.send_state('display_units')

def _on_refdata_change(self, *args):
if self.app._link_type.lower() == 'wcs':
if self.app._align_by.lower() == 'wcs':
self.display_units['image'] = 'deg'
else:
self.display_units['image'] = 'pix'
Expand Down
8 changes: 4 additions & 4 deletions jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ def __init__(self, *args, **kwargs):
self.subset_states = []
self.spectral_display_unit = None

link_type = getattr(self.app, '_link_type', None)
self.display_sky_coordinates = (link_type == 'wcs' and not self.multiselect)
align_by = getattr(self.app, '_align_by', None)
self.display_sky_coordinates = (align_by == 'wcs' and not self.multiselect)

def _on_link_update(self, *args):
"""When linking is changed pixels<>wcs, change display units of the
Expand All @@ -100,8 +100,8 @@ def _on_link_update(self, *args):
to the UI upon link change by calling _get_subset_definition, which
will re-determine how to display subset information."""

link_type = getattr(self.app, '_link_type', None)
self.display_sky_coordinates = (link_type == 'wcs')
align_by = getattr(self.app, '_align_by', None)
self.display_sky_coordinates = (align_by == 'wcs')

if self.subset_selected != self.subset_select.default_text:
self._get_subset_definition(*args)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def test_circle_recenter_linking(roi_class, subset_info, imviz_helper, image_2d_
# remove subsets and change link type to wcs
dc = imviz_helper.app.data_collection
dc.remove_subset_group(dc.subset_groups[0])
imviz_helper.link_data(link_type='wcs')
imviz_helper.link_data(align_by='wcs')
assert plugin.display_sky_coordinates # linking change should trigger change to True

# apply original subset. transform sky coord of original subset to new pixels
Expand Down
37 changes: 21 additions & 16 deletions jdaviz/configs/imviz/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import warnings
from copy import deepcopy

from astropy.utils import deprecated
import numpy as np
from glue.core.link_helpers import LinkSame

Expand Down Expand Up @@ -190,7 +191,7 @@ def load_data(self, data, data_label=None, show_in_viewer=True, **kwargs):
show_in_viewer = f"{self.app.config}-0"

if show_in_viewer:
linked_by_wcs = self.app._link_type == 'wcs'
linked_by_wcs = self.app._align_by == 'wcs'
if linked_by_wcs:
for applied_label, visible, is_wcs_only, has_wcs in zip(
applied_labels, applied_visible, layer_is_wcs_only, layer_has_wcs
Expand All @@ -211,7 +212,7 @@ def load_data(self, data, data_label=None, show_in_viewer=True, **kwargs):
else:
if 'Orientation' not in self.plugins.keys():
# otherwise plugin will handle linking automatically with DataCollectionAddMessage
self.link_data(link_type='wcs')
self.link_data(align_by='wcs')

# One input might load into multiple Data objects.
# NOTE: If the batch_load context manager was used, it will
Expand All @@ -223,35 +224,39 @@ def load_data(self, data, data_label=None, show_in_viewer=True, **kwargs):
if (has_wcs and linked_by_wcs) or not linked_by_wcs:
self.app.add_data_to_viewer(show_in_viewer, applied_label, visible=visible)

def link_data(self, link_type='pixels', wcs_fallback_scheme=None, wcs_use_affine=True):
def link_data(self, align_by='pixels', wcs_fallback_scheme=None, wcs_fast_approximation=True):
"""(Re)link loaded data in Imviz with the desired link type.
All existing links will be replaced.
Parameters
----------
link_type : {'pixels', 'wcs'}
align_by : {'pixels', 'wcs'}
Choose to link by pixels or WCS.
wcs_fallback_scheme : {None, 'pixels'}
If WCS linking failed, choose to fall back to linking by pixels or not at all.
This is only used when ``link_type='wcs'``.
This is only used when ``align_by='wcs'``.
Choosing `None` may result in some Imviz functionality not working properly.
wcs_use_affine : bool
wcs_fast_approximation : bool
Use an affine transform to represent the offset between images if possible
(requires that the approximation is accurate to within 1 pixel with the
full WCS transformations). If approximation fails, it will automatically
fall back to full WCS transformation. This is only used when ``link_type='wcs'``.
fall back to full WCS transformation. This is only used when ``align_by='wcs'``.
Affine approximation is much more performant at the cost of accuracy.
"""
from jdaviz.configs.imviz.plugins.orientation.orientation import link_type_msg_to_trait
from jdaviz.configs.imviz.plugins.orientation.orientation import align_by_msg_to_trait
plg = self.plugins["Orientation"]
plg._obj.wcs_use_fallback = wcs_fallback_scheme == 'pixels'
plg.wcs_use_affine = wcs_use_affine
plg.link_type = link_type_msg_to_trait[link_type]
plg.wcs_fast_approximation = wcs_fast_approximation
plg.align_by = align_by_msg_to_trait[align_by]

@deprecated(since="4.0", alternative="get_alignment_method")
def get_link_type(self, data_label_1, data_label_2):
return self.get_alignment_method(data_label_1, data_label_2)

def get_alignment_method(self, data_label_1, data_label_2):
"""Find the type of ``glue`` linking between the given
data labels. A link is bi-directional. If there are
more than 2 data in the collection, one of the given
Expand All @@ -264,7 +269,7 @@ def get_link_type(self, data_label_1, data_label_2):
Returns
-------
link_type : {'pixels', 'wcs', 'self'}
align_by : {'pixels', 'wcs', 'self'}
One of the link types accepted by the Orientation plugin
or ``'self'`` if the labels are identical.
Expand All @@ -277,23 +282,23 @@ def get_link_type(self, data_label_1, data_label_2):
if data_label_1 == data_label_2:
return "self"

link_type = None
align_by = None
for elink in self.app.data_collection.external_links:
elink_labels = (elink.data1.label, elink.data2.label)
if data_label_1 in elink_labels and data_label_2 in elink_labels:
if isinstance(elink, LinkSame): # Assumes WCS link never uses LinkSame
link_type = 'pixels'
align_by = 'pixels'
else: # If not pixels, must be WCS
link_type = 'wcs'
align_by = 'wcs'
break # Might have duplicate, just grab first match

if link_type is None:
if align_by is None:
avail_links = [f"({elink.data1.label}, {elink.data2.label})"
for elink in self.app.data_collection.external_links]
raise ValueError(f'{data_label_1} and {data_label_2} combo not found '
f'in data collection external links: {avail_links}')

return link_type
return align_by

def get_aperture_photometry_results(self):
"""Return aperture photometry results, if any.
Expand Down
6 changes: 3 additions & 3 deletions jdaviz/configs/imviz/plugins/footprints/footprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ def _ensure_sky(region):
return '', {path: region}

def _on_link_type_updated(self, msg=None):
self.is_pixel_linked = (getattr(self.app, '_link_type', None) == 'pixels' and
self.is_pixel_linked = (getattr(self.app, '_align_by', None) == 'pixels' and
len(self.app.data_collection) > 1)
# toggle visibility as necessary
self._on_is_active_changed()
Expand All @@ -193,10 +193,10 @@ def _on_link_type_updated(self, msg=None):
self._change_overlay(overlay_selected=choice, center_the_overlay=False)

def vue_link_by_wcs(self, *args):
# call other plugin so that other options (wcs_use_affine, wcs_use_fallback)
# call other plugin so that other options (wcs_fast_approximation, wcs_use_fallback)
# are retained. Remove this method if support for plotting footprints
# when pixel-linked is reintroduced.
self.app._jdaviz_helper.plugins['Orientation'].link_type = 'WCS'
self.app._jdaviz_helper.plugins['Orientation'].align_by = 'WCS'

def _ensure_first_overlay(self):
if not len(self._overlays):
Expand Down
Loading

0 comments on commit f4a6889

Please sign in to comment.