Skip to content

Commit

Permalink
WIP: Fix astrowidgets API
Browse files Browse the repository at this point in the history
More safeguards

fix multi viewer linked pan/zoom (or breaking it, not sure)

Fix CI
mainly by using more conventional ways to adjust the tests
take give back same result as main, so also undo the result changes.
Deleted unnecessary functions.

Better change log, add basic doc.
It bothers me that there is no user doc at all. I will add some basic info here.
Better doc is already a future ticket.

I do not need get_bottom_layer is needed

GWCS is also BaseHighLevelWCS, no need to check separately.

imviz.link_data API should error out sooner when subsets need to be cleared first.

Remove unnecessary check for viewer.data() especially since that might waste memory.

More code simplification. And clarify behavior more in doc.

Docstring nitpick

Nope did not work out as I hoped

I think I fixed aper phot
  • Loading branch information
pllim committed Nov 28, 2023
1 parent e95aefa commit 3e47e62
Show file tree
Hide file tree
Showing 21 changed files with 263 additions and 364 deletions.
11 changes: 9 additions & 2 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ Cubeviz
Imviz
^^^^^

- There is now option for image rotation in Links Control plugin.
This feature requires WCS linking. [#2179]

- Aperture photometry (previously "Imviz Simple Aperture Photometry") now supports batch mode. [#2465]

- Aperture photometry sum is now presented in scientific notation consistently. [#2530]
Expand All @@ -51,6 +54,12 @@ Cubeviz
Imviz
^^^^^

- Linking by WCS will now always generate a hidden reference data layer
without distortion. As a result, when WCS linked, the first loaded data
is no longer the reference data. Additionally, if data is distorted,
its distortion will show when linked by WCS. If there is also data without WCS,
it can no longer be displayed when WCS linked. [#2179]

Mosviz
^^^^^^

Expand Down Expand Up @@ -188,8 +197,6 @@ Cubeviz
Imviz
^^^^^

- Added viewer rotation support via Glue linking [#2179]

- The stretch histogram within plot options can now be popped-out into its own window. [#2314]

- vmin/vmax step size in the plot options plugin is now dynamic based on the full range of the
Expand Down
24 changes: 23 additions & 1 deletion docs/imviz/plugins.rst
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ Orientation

.. note::

This plugin was previous called "Links Control"
This plugin was previous called "Links Control".

This plugin is used to align image layers by pixels or sky (WCS) using
:func:`~jdaviz.configs.imviz.helper.link_image_data`.
Expand All @@ -119,6 +119,13 @@ performant at the cost of accuracy but should be accurate to within a pixel
for most cases. If approximation fails, WCS linking still automatically
falls back to full transformation.

Since Jdaviz v3.9, when linking by WCS, a hidden reference data layer
without distortion will be created and all the data would be linked to
it instead of the first loaded data. As a result, working in pixel
space when linked by WCS is not recommended. Additionally, any data
with distorted WCS would show as distorted on the display. Furthermore,
any data without WCS can no longer be shown in WCS linking mode.

For the best experience, it is recommended that you decide what kind of
link you want and set it at the beginning of your Imviz session,
rather than later.
Expand All @@ -134,6 +141,21 @@ From the API within the Jupyter notebook (if linking by WCS):
imviz.link_data(link_type='wcs')
.. _imviz-orientation-rotation:

Orientation: Image Rotation
===========================

When linked by WCS, sky rotation is also possible. You can choose from
presets (N-up, E-left/right) or provide your own sky angle.

.. warning::

Each rotation request created a new reference data layer in the background.
Just as in :ref:`imviz-import-data`, the performance would be impacted by
the number of active rotation layers you have; Only keep the desired rotation layer.
Note that the "default orientation" layer cannot be removed.

.. _imviz-compass:

Compass
Expand Down
2 changes: 1 addition & 1 deletion jdaviz/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ def _change_reference_data(self, new_refdata_label, viewer_id=None):
# adjust zoom to account for new refdata if both the
# old and new refdata are WCS-only layers
# (which also ensures zoom_level is already determined):
fov_sky_final = viewer._get_fov(new_refdata)
fov_sky_final = viewer._get_fov()
viewer.zoom(
float(fov_sky_final / fov_sky_init)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ def vue_recenter_subset(self, *args):

def _do_recentering(subset, subset_state):
try:
reg = _get_region_from_spatial_subset(self, subset_state) # noqa
reg = _get_region_from_spatial_subset(self, subset_state)
aperture = regions2aperture(reg)
data = self.dataset.selected_dc_item
comp = data.get_component(data.main_components[0])
Expand Down
22 changes: 6 additions & 16 deletions jdaviz/configs/imviz/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import astropy.units as u
from astropy.utils.exceptions import AstropyDeprecationWarning
from astropy.wcs.wcsapi import BaseHighLevelWCS
from gwcs.wcs import WCS as GWCS
from glue.core import BaseData
from glue.core.link_helpers import LinkSame
from glue.plugins.wcs_autolinking.wcs_autolinking import WCSLink, NoAffineApproximation
Expand Down Expand Up @@ -410,17 +409,6 @@ def layer_is_table_data(layer):
return isinstance(layer, BaseData) and layer.ndim == 1


def get_bottom_layer(viewer):
"""
Get the first-loaded image layer in Imviz.
"""
image_layers = [lyr.layer for lyr in viewer.layers
if lyr.visible and layer_is_image_data(lyr.layer)]
if not len(image_layers):
return None
return image_layers[0]


def get_wcs_only_layer_labels(app):
return [data.label for data in app.data_collection
if layer_is_wcs_only(data)]
Expand Down Expand Up @@ -519,7 +507,7 @@ def link_image_data(app, link_type='pixels', wcs_fallback_scheme=None, wcs_use_a
f"got {wcs_fallback_scheme}")
if link_type == 'wcs':
at_least_one_data_have_wcs = len([
hasattr(d, 'coords') and isinstance(d.coords, (BaseHighLevelWCS, GWCS))
hasattr(d, 'coords') and isinstance(d.coords, BaseHighLevelWCS)
for d in app.data_collection
]) > 0
if not at_least_one_data_have_wcs:
Expand Down Expand Up @@ -557,9 +545,7 @@ def link_image_data(app, link_type='pixels', wcs_fallback_scheme=None, wcs_use_a
old_link_type = getattr(app, '_link_type', None)
refdata, iref = get_reference_image_data(app)
# default reference layer is the first-loaded image:
default_reference_layer = get_bottom_layer(app._jdaviz_helper.default_viewer)
if default_reference_layer is None:
default_reference_layer = refdata
default_reference_layer = app.data_collection[0]

# if linking via WCS, add WCS-only reference data layer:
insert_base_wcs_layer = (
Expand Down Expand Up @@ -612,6 +598,10 @@ def link_image_data(app, link_type='pixels', wcs_fallback_scheme=None, wcs_use_a
# links already exist for this entry and we're not changing the type
continue

# We are not touching fake WCS layers in pixel linking.
if link_type == 'pixels' and data.meta.get('_WCS_ONLY'):
continue

ids1 = data.pixel_component_ids
new_links = []
try:
Expand Down
14 changes: 8 additions & 6 deletions jdaviz/configs/imviz/plugins/orientation/orientation.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import astropy.units as u
from jdaviz.configs.imviz.helper import (
link_image_data, get_bottom_layer
link_image_data
)
from jdaviz.configs.imviz.wcs_utils import (
get_compass_info, _get_rotated_nddata_from_label
Expand Down Expand Up @@ -243,6 +243,10 @@ def _update_link(self, msg={}):
self.linking_in_progress = False
self._update_layer_label_default()

# Clear previous zoom limits because they no longer mean anything.
for v in self.app._viewer_store.values():
v._prev_limits = None

def _on_subset_change(self, msg):
self.need_clear_subsets = len(self.app.data_collection.subset_groups) > 0

Expand All @@ -259,8 +263,7 @@ def vue_reset_astrowidget_markers(self, *args):
viewer.reset_markers()

def _get_wcs_angles(self):
viewer = self.app.get_viewer(self.viewer.selected)
first_loaded_image = get_bottom_layer(viewer)
first_loaded_image = self.app.data_collection[0]
degn, dege, flip = get_compass_info(
first_loaded_image.coords, first_loaded_image.shape
)[-3:]
Expand Down Expand Up @@ -302,8 +305,7 @@ def add_orientation(self, rotation_angle=None, east_left=None, label=None,
if wrt_data is None:
# if not specified, use first-loaded image layer as the
# default rotation:
viewer = self.app.get_viewer(self.viewer.selected)
wrt_data = get_bottom_layer(viewer)
wrt_data = self.app.data_collection[0]

# Default rotation is the same orientation as the original reference data:
degn = self._get_wcs_angles()[0]
Expand Down Expand Up @@ -383,7 +385,7 @@ def _refdata_change_available(self):
else:
is_subset = False
return (
ref_data is not None and len(viewer.data()) and
ref_data is not None and
len(self.orientation.selected) and len(self.viewer.selected) and
not is_subset
)
Expand Down
15 changes: 14 additions & 1 deletion jdaviz/configs/imviz/plugins/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,22 @@ class _ImvizMatchedZoomMixin(_MatchedZoomMixin):
disable_matched_zoom_in_other_viewer = False

def _is_matched_viewer(self, viewer):
# only match zooms in viewers that share reference data
return isinstance(viewer, BqplotImageView)

def _post_activate(self):
# NOTE: For Imviz only.
# Set the reference data in other viewers to be the same as the current viewer.
# If adding the data to the viewer, make sure it is not actually shown since the
# user didn't request it.
for viewer in self._iter_matched_viewers(include_self=False):
if self.viewer.state.reference_data not in viewer.state.layers_data:
viewer.add_data(self.viewer.state.reference_data)
for layer in viewer.state.layers:
if layer.layer is self.viewer.state.reference_data:
layer.visible = False
break
viewer.state.reference_data = self.viewer.state.reference_data


@viewer_tool
class ImagePanZoom(PanZoom):
Expand Down
37 changes: 16 additions & 21 deletions jdaviz/configs/imviz/plugins/viewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,43 +305,38 @@ def get_link_type(self, data_label):

return link_type

def _get_fov(self, data):
def _get_fov(self):
data = self.state.reference_data
if self.jdaviz_app._link_type != "wcs" or data.coords is None:
return

# compute the mean of the height and width of the
# viewer's FOV on ``data`` in world units:
x_corners = [
self.state.x_min,
self.state.x_max,
self.state.x_min,
self.state.x_max
self.state.x_min
]
y_corners = [
self.state.y_min,
self.state.y_min,
self.state.y_max,
self.state.y_max
]

y_corners, x_corners = self._get_real_xy(
data, x_corners, y_corners
)[:2]
sky_corners = data.coords.pixel_to_world(x_corners * u.pix, y_corners * u.pix)
sky_corners = data.coords.pixel_to_world(x_corners, y_corners)
height_sky = abs(sky_corners[0].separation(sky_corners[2]))
width_sky = abs(sky_corners[0].separation(sky_corners[1]))
fov_sky = u.Quantity([height_sky, width_sky]).mean()
return fov_sky

def _get_center_skycoord(self, data=None):
if data is None:
data = self.state.reference_data
# get SkyCoord for the center of ``data`` in this viewer:
width = self.state.x_max - self.state.x_min
height = self.state.y_max - self.state.y_min
x_cen = self.state.x_min + (width * 0.5)
y_cen = self.state.y_min + (height * 0.5)
x_cen, y_cen = self._get_real_xy(
data, x_cen, y_cen
)[:2]
sky_cen = data.coords.pixel_to_world(
x_cen * u.pix, y_cen * u.pix
)
return sky_cen
x_cen = (self.state.x_min + self.state.x_max) * 0.5
y_cen = (self.state.y_min + self.state.y_max) * 0.5

if (self.jdaviz_app._link_type == "wcs" or data is None
or data.label == self.state.reference_data.label):
return self.state.reference_data.coords.pixel_to_world(x_cen, y_cen)

if data.coords is not None:
return data.coords.pixel_to_world(x_cen, y_cen)
3 changes: 1 addition & 2 deletions jdaviz/configs/imviz/tests/test_astrowidgets_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ def test_center_on_pix(self):
self.viewer.center_on((0, 0))
assert_allclose((self.viewer.state.x_min, self.viewer.state.x_max,
self.viewer.state.y_min, self.viewer.state.y_max),
[lim if i > 1 else lim + 1
for i, lim in enumerate(expected_position)], rtol=rtol)
[-5.75, 5.25, -5.75, 5.25], rtol=rtol)

# Centering by sky on second data.
self.viewer.blink_once()
Expand Down
24 changes: 12 additions & 12 deletions jdaviz/configs/imviz/tests/test_delete_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ def test_delete_with_subset_wcs(self):
self.imviz.link_data(link_type='wcs', wcs_fallback_scheme=None, error_on_fail=True)

# Add a subset
reg = CirclePixelRegion(PixCoord(2, 2), 3)
reg = CirclePixelRegion(PixCoord(2, 2), 3).to_sky(self.wcs_1)
self.imviz.load_regions(reg)

reg = RectanglePixelRegion(PixCoord(1, 1), 2, 2)
reg = RectanglePixelRegion(PixCoord(1, 1), 2, 2).to_sky(self.wcs_1)
self.imviz.load_regions(reg)

assert len(self.imviz.app.data_collection.subset_groups) == 2
Expand All @@ -47,13 +47,13 @@ def test_delete_with_subset_wcs(self):
subset1 = self.imviz.app.data_collection.subset_groups[0]
subset2 = self.imviz.app.data_collection.subset_groups[1]
assert subset1.subset_state.xatt.parent.label == "has_wcs_1[SCI,1]"
assert_allclose(subset1.subset_state.center(), (2.25, 2.25))
assert_allclose(subset1.subset_state.center(), (2, 2))

assert subset2.subset_state.xatt.parent.label == "has_wcs_1[SCI,1]"
assert_allclose(subset2.subset_state.roi.xmin, 0.25)
assert_allclose(subset2.subset_state.roi.ymin, 0.25)
assert_allclose(subset2.subset_state.roi.xmax, 2.25)
assert_allclose(subset2.subset_state.roi.ymax, 2.25)
assert_allclose(subset2.subset_state.roi.xmin, 0, atol=1e-6)
assert_allclose(subset2.subset_state.roi.ymin, 0, atol=1e-6)
assert_allclose(subset2.subset_state.roi.xmax, 2)
assert_allclose(subset2.subset_state.roi.ymax, 2)

# We have to remove the data from the viewer before deleting the data from the app.
self.imviz.app.remove_data_from_viewer("imviz-0", "has_wcs_1[SCI,1]")
Expand All @@ -64,10 +64,10 @@ def test_delete_with_subset_wcs(self):

# Check that the reparenting and coordinate recalculations happened
assert subset1.subset_state.xatt.parent.label == "has_wcs_2[SCI,1]"
assert_allclose(subset1.subset_state.center(), (3.25, 2.25))
assert_allclose(subset1.subset_state.center(), (3, 2))

assert subset2.subset_state.xatt.parent.label == "has_wcs_2[SCI,1]"
assert_allclose(subset2.subset_state.roi.xmin, 1.25, atol=1e-6)
assert_allclose(subset2.subset_state.roi.ymin, 0.25, atol=1e-6)
assert_allclose(subset2.subset_state.roi.xmax, 3.25, atol=1e-6)
assert_allclose(subset2.subset_state.roi.ymax, 2.25, atol=1e-6)
assert_allclose(subset2.subset_state.roi.xmin, 1)
assert_allclose(subset2.subset_state.roi.ymin, 0, atol=1e-6)
assert_allclose(subset2.subset_state.roi.xmax, 3)
assert_allclose(subset2.subset_state.roi.ymax, 2)
Loading

0 comments on commit 3e47e62

Please sign in to comment.