Skip to content

Commit

Permalink
BUG: Imviz bug fixes ported from spacetelescope#1340
Browse files Browse the repository at this point in the history
  • Loading branch information
pllim committed Jun 10, 2022
1 parent 6eea29d commit 4165940
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 38 deletions.
9 changes: 9 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ Imviz
linked to the data collection, resulting in Imviz unusable until
the data are re-linked manually. [#1365]

- Fixed a bug where coordinates display erroneously showing info from
the reference image even when it is not visible. [#1392]

- Fixed a bug where Compass zoom box is wrong when the second image
is rotated w.r.t. the reference image and they are linked by WCS. [#1392]

- Fixed a bug where Line Profile might crash when the second image
is rotated w.r.t. the reference image and they are linked by WCS. [#1392]

Mosviz
^^^^^^

Expand Down
19 changes: 14 additions & 5 deletions jdaviz/configs/imviz/plugins/line_profile_xy/line_profile_xy.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,14 @@ def vue_draw_plot(self, *args, **kwargs):
self.reset_results()
return

x_min, y_min, x_max, y_max = viewer._get_zoom_limits(data)
xy_limits = viewer._get_zoom_limits(data)
x_limits = xy_limits[:, 0]
y_limits = xy_limits[:, 1]
x_min = x_limits.min()
x_max = x_limits.max()
y_min = y_limits.min()
y_max = y_limits.max()

comp = data.get_component(data.main_components[0])
if comp.units:
y_label = comp.units
Expand Down Expand Up @@ -101,8 +108,9 @@ def vue_draw_plot(self, *args, **kwargs):
y_min = max(int(y_min), 0)
y_max = min(int(y_max), ny)
zoomed_data_x = comp.data[y_min:y_max, x]
line_x.scales['y'].min = zoomed_data_x.min() * 0.95
line_x.scales['y'].max = zoomed_data_x.max() * 1.05
if zoomed_data_x.size > 0:
line_x.scales['y'].min = zoomed_data_x.min() * 0.95
line_x.scales['y'].max = zoomed_data_x.max() * 1.05

fig_y.title = f'Y={y}'
fig_y.title_style = {'font-size': '12px'}
Expand All @@ -119,8 +127,9 @@ def vue_draw_plot(self, *args, **kwargs):
x_min = max(int(x_min), 0)
x_max = min(int(x_max), nx)
zoomed_data_y = comp.data[y, x_min:x_max]
line_y.scales['y'].min = zoomed_data_y.min() * 0.95
line_y.scales['y'].max = zoomed_data_y.max() * 1.05
if zoomed_data_y.size > 0:
line_y.scales['y'].min = zoomed_data_y.min() * 0.95
line_y.scales['y'].max = zoomed_data_y.max() * 1.05

self.line_plot_across_x = fig_x
self.line_plot_across_y = fig_y
Expand Down
26 changes: 17 additions & 9 deletions jdaviz/configs/imviz/plugins/viewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,14 @@ def __init__(self, *args, **kwargs):
def on_mouse_or_key_event(self, data):

# Find visible layers
visible_layers = [layer for layer in self.state.layers if layer.visible]
visible_layers = [layer for layer in self.state.layers
if (layer.visible and layer_is_image_data(layer.layer))]

if len(visible_layers) == 0:
return

active_layer = visible_layers[-1]

if self.label_mouseover is None:
if 'g-coords-info' in self.session.application._tools:
self.label_mouseover = self.session.application._tools['g-coords-info']
Expand All @@ -97,7 +100,7 @@ def on_mouse_or_key_event(self, data):

# Extract first dataset from visible layers and use this for coordinates - the choice
# of dataset shouldn't matter if the datasets are linked correctly
image = visible_layers[0].layer
image = active_layer.layer

# Extract data coordinates - these are pixels in the reference image
x = data['domain']['x']
Expand Down Expand Up @@ -125,8 +128,8 @@ def on_mouse_or_key_event(self, data):
# of how to display values when multiple datasets are present.
if (x > -0.5 and y > -0.5
and x < image.shape[1] - 0.5 and y < image.shape[0] - 0.5
and hasattr(visible_layers[0], 'attribute')):
attribute = visible_layers[0].attribute
and hasattr(active_layer, 'attribute')):
attribute = active_layer.attribute
value = image.get_data(attribute)[int(round(y)), int(round(x))]
unit = image.get_component(attribute).units
self.label_mouseover.value = f'{value:+10.5e} {unit}'
Expand All @@ -151,7 +154,7 @@ def on_mouse_or_key_event(self, data):

elif key_pressed == 'l':
# Same data as mousemove above.
image = visible_layers[0].layer
image = active_layer.layer
x = data['domain']['x']
y = data['domain']['y']
if x is None or y is None: # Out of bounds
Expand Down Expand Up @@ -243,18 +246,23 @@ def _get_real_xy(self, image, x, y):
return x, y, coords_status

def _get_zoom_limits(self, image):
"""Return ``(x_min, y_min, x_max, y_max)`` for given image.
"""Return a list of ``(x, y)`` that defines four corners of
the zoom box for a given image.
This is needed because viewer values are only based on reference
image, which can be inaccurate if given image is dithered and
they are linked by WCS.
"""
if data_has_valid_wcs(image) and self.get_link_type(image.label) == 'wcs':
# Convert X,Y from reference data to the one we are actually seeing.
x = image.coords.world_to_pixel(self.state.reference_data.coords.pixel_to_world(
(self.state.x_min, self.state.x_max), (self.state.y_min, self.state.y_max)))
zoom_limits = (x[0][0], x[1][0], x[0][1], x[1][1])
(self.state.x_min, self.state.x_min, self.state.x_max, self.state.x_max),
(self.state.y_min, self.state.y_max, self.state.y_max, self.state.y_min)))
zoom_limits = np.array(list(zip(x[0], x[1])))
else:
zoom_limits = (self.state.x_min, self.state.y_min, self.state.x_max, self.state.y_max)
zoom_limits = np.array(((self.state.x_min, self.state.y_min),
(self.state.x_min, self.state.y_max),
(self.state.x_max, self.state.y_max),
(self.state.x_max, self.state.y_min)))
return zoom_limits

def set_compass(self, image):
Expand Down
72 changes: 55 additions & 17 deletions jdaviz/configs/imviz/tests/test_linking.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
from regions import PixCoord, CirclePixelRegion

from jdaviz.configs.imviz.helper import get_reference_image_data
from jdaviz.configs.imviz.tests.utils import BaseImviz_WCS_NoWCS, BaseImviz_WCS_WCS
from jdaviz.configs.imviz.tests.utils import (
BaseImviz_WCS_NoWCS, BaseImviz_WCS_WCS, BaseImviz_WCS_GWCS)


class BaseLinkHandler:
Expand All @@ -30,24 +31,20 @@ def test_wcslink_fallback_pixels(self):
assert self.viewer.get_link_type('has_wcs[SCI,1]') == 'self'
assert self.viewer.get_link_type('no_wcs[SCI,1]') == 'pixels'

# Also check the coordinates display
# Also check the coordinates display: Last loaded is on top.

self.viewer.on_mouse_or_key_event({'event': 'mousemove', 'domain': {'x': 0, 'y': 0}})
assert self.viewer.label_mouseover.pixel == 'x=00.0 y=00.0'
assert self.viewer.label_mouseover.value == '+0.00000e+00 '
assert self.viewer.label_mouseover.world_ra_deg == '337.5202808000'
assert self.viewer.label_mouseover.world_dec_deg == '-20.8333330600'

# Not sure why but need one extra blink to work properly.
# This does not happen when we load real data from files.
self.viewer.blink_once()
assert self.viewer.label_mouseover.world_ra_deg == ''
assert self.viewer.label_mouseover.world_dec_deg == ''

self.viewer.on_mouse_or_key_event({'event': 'keydown', 'key': 'b',
'domain': {'x': 0, 'y': 0}})
assert self.viewer.label_mouseover.pixel == 'x=00.0 y=00.0'
assert self.viewer.label_mouseover.value == '+0.00000e+00 '
assert self.viewer.label_mouseover.world_ra_deg == ''
assert self.viewer.label_mouseover.world_dec_deg == ''
assert self.viewer.label_mouseover.world_ra_deg == '337.5202808000'
assert self.viewer.label_mouseover.world_dec_deg == '-20.8333330600'

def test_wcslink_nofallback_noerror(self):
self.imviz.link_data(link_type='wcs', wcs_fallback_scheme=None)
Expand Down Expand Up @@ -114,21 +111,17 @@ def test_wcslink_affine_with_extras(self):
assert_allclose((self.viewer.state.x_min, self.viewer.state.y_min,
self.viewer.state.x_max, self.viewer.state.y_max), ans)

# Also check the coordinates display
# Also check the coordinates display: Last loaded is on top.

self.viewer.on_mouse_or_key_event({'event': 'mousemove', 'domain': {'x': 0, 'y': 0}})
assert self.viewer.label_mouseover.pixel == 'x=00.0 y=00.0'
assert self.viewer.label_mouseover.pixel == 'x=01.0 y=-0.0'
assert self.viewer.label_mouseover.value == '+1.00000e+00 '
assert self.viewer.label_mouseover.world_ra_deg == '337.5202808000'
assert self.viewer.label_mouseover.world_dec_deg == '-20.8333330600'

# Not sure why but need one extra blink to work properly.
# This does not happen when we load real data from files.
self.viewer.blink_once()

self.viewer.on_mouse_or_key_event({'event': 'keydown', 'key': 'b',
'domain': {'x': 0, 'y': 0}})
assert self.viewer.label_mouseover.pixel == 'x=01.0 y=-0.0'
assert self.viewer.label_mouseover.pixel == 'x=00.0 y=00.0'
assert self.viewer.label_mouseover.value == '+1.00000e+00 '
assert self.viewer.label_mouseover.world_ra_deg == '337.5202808000'
assert self.viewer.label_mouseover.world_dec_deg == '-20.8333330600'
Expand All @@ -155,6 +148,51 @@ def test_invalid_inputs(self):
self.viewer.get_link_type('foo')


class TestLink_WCS_GWCS(BaseImviz_WCS_GWCS):

def test_wcslink_rotated(self):
# FITS WCS will be reference, GWCS is rotated, no_wcs linked by pixel to ref.
self.imviz.link_data(link_type='wcs', error_on_fail=True)

# The zoom box for GWCS is now a rotated rombus.
fits_wcs_zoom_limits = self.viewer._get_zoom_limits(
self.imviz.app.data_collection['fits_wcs[DATA]'])
gwcs_zoom_limits = self.viewer._get_zoom_limits(
self.imviz.app.data_collection['gwcs[DATA]'])
no_wcs_zoom_limits = self.viewer._get_zoom_limits(
self.imviz.app.data_collection['no_wcs'])
assert_allclose(fits_wcs_zoom_limits,
((-0.972136, 0.027864), (-0.972136, 8.972136),
(7.972136, 8.972136), (7.972136, 0.027864)), rtol=1e-5)
assert_allclose(gwcs_zoom_limits,
((3.245117, 10.549265), (10.688389, 4.95208),
(6.100057, -2.357782), (-1.343215, 3.239403)), rtol=1e-5)
assert_allclose(no_wcs_zoom_limits, fits_wcs_zoom_limits)

# Also check the coordinates display: Last loaded is on top.
# Cycle order: no_wcs, FITS WCS, GWCS

self.viewer.on_mouse_or_key_event({'event': 'mousemove', 'domain': {'x': 0, 'y': 0}})
assert self.viewer.label_mouseover.pixel == 'x=00.0 y=00.0'
assert self.viewer.label_mouseover.value == '+1.00000e+00'
assert self.viewer.label_mouseover.world_ra_deg == ''
assert self.viewer.label_mouseover.world_dec_deg == ''

self.viewer.on_mouse_or_key_event({'event': 'keydown', 'key': 'b',
'domain': {'x': 0, 'y': 0}})
assert self.viewer.label_mouseover.pixel == 'x=00.0 y=00.0'
assert self.viewer.label_mouseover.value == '+1.00000e+00'
assert self.viewer.label_mouseover.world_ra_deg == '3.5817255823'
assert self.viewer.label_mouseover.world_dec_deg == '-30.3920580740'

self.viewer.on_mouse_or_key_event({'event': 'keydown', 'key': 'b',
'domain': {'x': 0, 'y': 0}})
assert self.viewer.label_mouseover.pixel == 'x=02.7 y=09.8'
assert self.viewer.label_mouseover.value == ''
assert self.viewer.label_mouseover.world_ra_deg == '3.5817255823'
assert self.viewer.label_mouseover.world_dec_deg == '-30.3920580740'


def test_imviz_no_data(imviz_helper):
with pytest.raises(ValueError, match='No valid reference data'):
get_reference_image_data(imviz_helper.app)
Expand Down
63 changes: 63 additions & 0 deletions jdaviz/configs/imviz/tests/utils.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import gwcs
import numpy as np
import pytest
from astropy import units as u
from astropy.coordinates import ICRS
from astropy.io import fits
from astropy.modeling import models
from astropy.nddata import NDData
from astropy.wcs import WCS
from gwcs import coordinate_frames as cf

__all__ = ['BaseImviz_WCS_NoWCS', 'BaseImviz_WCS_WCS']

Expand Down Expand Up @@ -88,3 +94,60 @@ def setup_class(self, imviz_helper):
# Since we are not really displaying, need this to test zoom.
self.viewer.shape = (100, 100)
self.viewer.state._set_axes_aspect_ratio(1)


class BaseImviz_WCS_GWCS:
@pytest.fixture(autouse=True)
def setup_class(self, imviz_helper):
arr = np.zeros((10, 8)) # (ny, nx)
arr[0, 0] = 1 # Bright corner for sanity check

# FITS WCS that is adapted from HST/ACS without the distortion.
w_fits = WCS({'WCSAXES': 2, 'NAXIS1': 8, 'NAXIS2': 10,
'CRPIX1': 5.0, 'CRPIX2': 5.0,
'PC1_1': -1.14852e-05, 'PC1_2': 7.01477e-06,
'PC2_1': 7.75765e-06, 'PC2_2': 1.20927e-05,
'CDELT1': 1.0, 'CDELT2': 1.0,
'CUNIT1': 'deg', 'CUNIT2': 'deg',
'CTYPE1': 'RA---TAN', 'CTYPE2': 'DEC--TAN',
'CRVAL1': 3.581704851882, 'CRVAL2': -30.39197867265,
'LONPOLE': 180.0, 'LATPOLE': -30.39197867265,
'MJDREF': 0.0, 'RADESYS': 'ICRS'})

# GWCS that is adapted from its Getting Started.
shift_by_crpix = models.Shift(-(5 - 1) * u.pix) & models.Shift(-(5 - 1) * u.pix)
matrix = np.array([[1.290551569736E-05, 5.9525007864732E-06],
[5.0226382102765E-06, -1.2644844123757E-05]])
rotation = models.AffineTransformation2D(matrix * u.deg, translation=[0, 0] * u.deg)
rotation.input_units_equivalencies = {"x": u.pixel_scale(1 * (u.deg / u.pix)),
"y": u.pixel_scale(1 * (u.deg / u.pix))}
rotation.inverse = models.AffineTransformation2D(np.linalg.inv(matrix) * u.pix,
translation=[0, 0] * u.pix)
rotation.inverse.input_units_equivalencies = {"x": u.pixel_scale(1 * (u.pix / u.deg)),
"y": u.pixel_scale(1 * (u.pix / u.deg))}
tan = models.Pix2Sky_TAN()
celestial_rotation = models.RotateNative2Celestial(
3.581704851882 * u.deg, -30.39197867265 * u.deg, 180 * u.deg)
det2sky = shift_by_crpix | rotation | tan | celestial_rotation
det2sky.name = "linear_transform"
detector_frame = cf.Frame2D(name="detector", axes_names=("x", "y"), unit=(u.pix, u.pix))
sky_frame = cf.CelestialFrame(reference_frame=ICRS(), name='icrs', unit=(u.deg, u.deg))
pipeline = [(detector_frame, det2sky), (sky_frame, None)]
w_gwcs = gwcs.WCS(pipeline)

# Load data into Imviz:
# 1. Data with FITS WCS and unit.
# 2. Data with GWCS (rotated w.r.t. FITS WCS) and no unit.
# 3. Data without WCS nor unit.
imviz_helper.load_data(NDData(arr, wcs=w_fits, unit='electron/s'), data_label='fits_wcs')
imviz_helper.load_data(NDData(arr, wcs=w_gwcs), data_label='gwcs')
imviz_helper.load_data(arr, data_label='no_wcs')

self.wcs_1 = w_fits
self.wcs_2 = w_gwcs
self.imviz = imviz_helper
self.viewer = imviz_helper.default_viewer

# Since we are not really displaying, need this to test zoom.
self.viewer.shape = (100, 100)
self.viewer.state._set_axes_aspect_ratio(1)
13 changes: 6 additions & 7 deletions jdaviz/configs/imviz/wcs_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import matplotlib.pyplot as plt
import numpy as np
from astropy.coordinates import SkyCoord
from matplotlib.patches import Rectangle
from matplotlib.patches import Polygon

__all__ = ['get_compass_info', 'draw_compass_mpl']

Expand Down Expand Up @@ -182,9 +182,10 @@ def draw_compass_mpl(image, orig_shape=None, wcs=None, show=True, zoom_limits=No
show : bool
Display the plot.
zoom_limits : tuple of float or None
zoom_limits : ndarray or None
If not `None`, also draw a rectangle to represent the
current zoom limits in the form of ``(x1, y1, x2, y2)``.
current zoom limits in the form of list of ``(x, y)``
representing the four corners of the zoom box.
kwargs : dict
Keywords for ``matplotlib.pyplot.imshow``.
Expand Down Expand Up @@ -238,10 +239,8 @@ def draw_compass_mpl(image, orig_shape=None, wcs=None, show=True, zoom_limits=No
color='yellow', fontsize=16, va='center', ha='center')

if zoom_limits is not None:
zx1, zy1, zx2, zy2 = zoom_limits
rect = Rectangle((zx1, zy1), zx2 - zx1, zy2 - zy1,
linewidth=1.5, edgecolor='r', facecolor='none')
ax.add_patch(rect)
ax.add_patch(Polygon(
zoom_limits, closed=True, linewidth=1.5, edgecolor='r', facecolor='none'))

if show:
plt.draw()
Expand Down

0 comments on commit 4165940

Please sign in to comment.