diff --git a/CHANGES.rst b/CHANGES.rst index 0538c1666f..bfcaf0b91e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -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 ^^^^^^ diff --git a/jdaviz/configs/imviz/plugins/line_profile_xy/line_profile_xy.py b/jdaviz/configs/imviz/plugins/line_profile_xy/line_profile_xy.py index e934eabc0e..28baeffa02 100644 --- a/jdaviz/configs/imviz/plugins/line_profile_xy/line_profile_xy.py +++ b/jdaviz/configs/imviz/plugins/line_profile_xy/line_profile_xy.py @@ -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 @@ -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'} @@ -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 diff --git a/jdaviz/configs/imviz/plugins/viewers.py b/jdaviz/configs/imviz/plugins/viewers.py index 4a51e3ae9b..eb3eade2d5 100644 --- a/jdaviz/configs/imviz/plugins/viewers.py +++ b/jdaviz/configs/imviz/plugins/viewers.py @@ -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'] @@ -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'] @@ -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}' @@ -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 @@ -243,7 +246,8 @@ 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. @@ -251,10 +255,14 @@ def _get_zoom_limits(self, image): 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): diff --git a/jdaviz/configs/imviz/tests/test_linking.py b/jdaviz/configs/imviz/tests/test_linking.py index cacca50de1..9f08ef9415 100644 --- a/jdaviz/configs/imviz/tests/test_linking.py +++ b/jdaviz/configs/imviz/tests/test_linking.py @@ -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: @@ -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) @@ -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' @@ -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 electron / s' + 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) diff --git a/jdaviz/configs/imviz/tests/utils.py b/jdaviz/configs/imviz/tests/utils.py index 8d674c30a8..f79e68404c 100644 --- a/jdaviz/configs/imviz/tests/utils.py +++ b/jdaviz/configs/imviz/tests/utils.py @@ -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'] @@ -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) diff --git a/jdaviz/configs/imviz/wcs_utils.py b/jdaviz/configs/imviz/wcs_utils.py index e244fae1a6..4d6444a68f 100644 --- a/jdaviz/configs/imviz/wcs_utils.py +++ b/jdaviz/configs/imviz/wcs_utils.py @@ -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'] @@ -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``. @@ -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()