Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TST: Increase Imviz rotation test coverage #2712

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion jdaviz/configs/imviz/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,10 @@ def get_link_type(self, data_label_1, data_label_2):
break # Might have duplicate, just grab first match

if link_type 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 '
'in data collection external links')
f'in data collection external links: {avail_links}')

return link_type

Expand Down
37 changes: 35 additions & 2 deletions jdaviz/configs/imviz/tests/test_delete_data.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import numpy as np
import pytest
from astropy.coordinates import Angle
from astropy.nddata import NDData
from astropy.tests.helper import assert_quantity_allclose
from numpy.testing import assert_allclose
from regions import PixCoord, CirclePixelRegion, RectanglePixelRegion
from regions import PixCoord, CirclePixelRegion, RectanglePixelRegion, EllipsePixelRegion

from jdaviz.configs.imviz.tests.utils import BaseImviz_WCS_WCS
from jdaviz.configs.imviz.tests.utils import BaseImviz_WCS_WCS, BaseImviz_WCS_GWCS


class TestDeleteData(BaseImviz_WCS_WCS):
Expand Down Expand Up @@ -71,3 +74,33 @@ def test_delete_with_subset_wcs(self):
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)


@pytest.mark.xfail(reason="FIXME: JDAT-3958")
class TestDeleteWCSLayerWithSubset(BaseImviz_WCS_GWCS):
pllim marked this conversation as resolved.
Show resolved Hide resolved
"""Regression test for https://jira.stsci.edu/browse/JDAT-3958"""
def test_delete_wcs_layer_with_subset(self):
lc_plugin = self.imviz.plugins['Orientation']
lc_plugin.link_type = 'WCS'

# Should automatically be applied as reference to first viewer.
lc_plugin._obj.create_north_up_east_left(set_on_create=True)

# Create a rotated ellipse.
reg = EllipsePixelRegion(
PixCoord(3.5, 4.5), width=2, height=5, angle=Angle(30, 'deg')).to_sky(self.wcs_1)
self.imviz.load_regions(reg)

# Switch back to Default Orientation.
self.imviz.app._change_reference_data("Default orientation")

# Delete N-up E-left reference data from GUI.
self.imviz.app.vue_data_item_remove({"item_name": "North-up, East-left"})

# Make sure rotated ellipse is still the same as before.
out_reg_d = self.imviz.app.get_subsets(include_sky_region=True)['Subset 1'][0]['sky_region']
assert_allclose(reg.center.ra.deg, out_reg_d.center.ra.deg)
assert_allclose(reg.center.dec.deg, out_reg_d.center.dec.deg)
assert_quantity_allclose(reg.height, out_reg_d.height)
assert_quantity_allclose(reg.width, out_reg_d.width)
assert_quantity_allclose(reg.angle, out_reg_d.angle)
153 changes: 138 additions & 15 deletions jdaviz/configs/imviz/tests/test_orientation.py
Original file line number Diff line number Diff line change
@@ -1,31 +1,154 @@
import pytest

from astropy.table import Table
from numpy.testing import assert_allclose

from jdaviz.configs.imviz.tests.utils import BaseImviz_WCS_WCS


class TestLinksControl(BaseImviz_WCS_WCS):
def test_plugin(self):
lc_plugin = self.imviz.app.get_tray_item_from_name('imviz-orientation')
class TestDefaultOrientation(BaseImviz_WCS_WCS):
def test_affine_reset_and_linktype(self):
lc_plugin = self.imviz.plugins['Orientation']

lc_plugin.link_type.selected = 'WCS'
lc_plugin.link_type = 'WCS'
lc_plugin.wcs_use_affine = False
lc_plugin.link_type.selected = 'Pixels'
assert self.imviz.get_link_type("Default orientation", "has_wcs_2[SCI,1]") == "wcs"

# wcs_use_affine should revert/default to True
# wcs_use_affine should revert/default to True when change back to Pixels.
lc_plugin.link_type = 'Pixels'
assert lc_plugin.wcs_use_affine is True
assert self.imviz.get_link_type("has_wcs_1[SCI,1]", "has_wcs_2[SCI,1]") == "pixels"

assert self.imviz.get_link_type("has_wcs_1[SCI,1]", "has_wcs_1[SCI,1]") == "self"

with pytest.raises(ValueError, match=".*combo not found"):
self.imviz.get_link_type("has_wcs_1[SCI,1]", "foo")

# adding markers should disable changing linking from both UI and API
assert lc_plugin.need_clear_astrowidget_markers is False
def test_astrowidgets_markers_disable_relinking(self):
lc_plugin = self.imviz.plugins['Orientation']
lc_plugin.link_type = 'Pixels'

# Adding markers should disable changing linking from both UI and API.
assert lc_plugin._obj.need_clear_astrowidget_markers is False
tbl = Table({'x': (0, 0), 'y': (0, 1)})
self.viewer.add_markers(tbl, marker_name='xy_markers')

assert lc_plugin.need_clear_astrowidget_markers is True
assert lc_plugin._obj.need_clear_astrowidget_markers is True
with pytest.raises(ValueError, match="cannot change linking"):
lc_plugin.link_type.selected = 'WCS'
assert lc_plugin.link_type.selected == 'Pixels'
lc_plugin.link_type = 'WCS'
assert lc_plugin.link_type == 'Pixels'

lc_plugin._obj.vue_reset_astrowidget_markers()

assert lc_plugin._obj.need_clear_astrowidget_markers is False
lc_plugin.link_type = 'WCS'

def test_markers_plugin_recompute_positions_pixels_to_wcs(self):
pllim marked this conversation as resolved.
Show resolved Hide resolved
lc_plugin = self.imviz.plugins['Orientation']
lc_plugin.link_type = 'Pixels'

# Blink to second image, if we have to.
if self.viewer.top_visible_data_label != "has_wcs_2[SCI,1]":
self.viewer.blink_once()
Comment on lines +49 to +51
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this ever happen (ie is it random)? Maybe just put an assert statement here to ensure that the top layer is as expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I have multiple test methods, I want to guard against chances when they are run out of order (could be possible if xdist is involved).


label_mouseover = self.imviz.app.session.application._tools['g-coords-info']
mp = self.imviz.plugins['Markers']

with mp.as_active():
# (1, 0) on second image but same sky coordinates as (0, 0) on first.
label_mouseover._viewer_mouse_event(
self.viewer, {'event': 'mousemove', 'domain': {'x': 1, 'y': 0}})
mp._obj._on_viewer_key_event(self.viewer, {'event': 'keydown', 'key': 'm'})

# (0, 0) on first image.
self.viewer.blink_once()
label_mouseover._viewer_mouse_event(
self.viewer, {'event': 'mousemove', 'domain': {'x': 0, 'y': 0}})
mp._obj._on_viewer_key_event(self.viewer, {'event': 'keydown', 'key': 'm'})

lc_plugin.link_type = 'WCS'

# Both marks stay the same in sky, so this means X and Y w.r.t. reference
# same on both entries.
# FIXME: 0.25 offset introduced by fake WCS layer (remove AssertionError).
# https://jira.stsci.edu/browse/JDAT-4256
with pytest.raises(AssertionError):
assert_allclose(mp._obj.marks["imviz-0"].x, 0)
with pytest.raises(AssertionError):
assert_allclose(mp._obj.marks["imviz-0"].y, 0)

mp.clear_table()

def test_markers_plugin_recompute_positions_wcs_to_pixels(self):
lc_plugin = self.imviz.plugins['Orientation']
lc_plugin.link_type = 'WCS'

# Blink to second image, if we have to.
if self.viewer.top_visible_data_label != "has_wcs_2[SCI,1]":
self.viewer.blink_once()

label_mouseover = self.imviz.app.session.application._tools['g-coords-info']
mp = self.imviz.plugins['Markers']

with mp.as_active():
# (0, 0) on second image, but linked by WCS.
label_mouseover._viewer_mouse_event(
self.viewer, {'event': 'mousemove', 'domain': {'x': 0, 'y': 0}})
mp._obj._on_viewer_key_event(self.viewer, {'event': 'keydown', 'key': 'm'})

# (0, 0) on first image.
self.viewer.blink_once()
label_mouseover._viewer_mouse_event(
self.viewer, {'event': 'mousemove', 'domain': {'x': 0, 'y': 0}})
mp._obj._on_viewer_key_event(self.viewer, {'event': 'keydown', 'key': 'm'})

lc_plugin.link_type = 'Pixels'

# Both marks now get separated, so this means X and Y w.r.t. reference
# are different on both entries.
# FIXME: 0.25 offset introduced by fake WCS layer (remove AssertionError).
# https://jira.stsci.edu/browse/JDAT-4256
with pytest.raises(AssertionError):
assert_allclose(mp._obj.marks["imviz-0"].x, [1, 0])
with pytest.raises(AssertionError):
assert_allclose(mp._obj.marks["imviz-0"].y, 0)

mp.clear_table()


class TestNonDefaultOrientation(BaseImviz_WCS_WCS):
def test_N_up_multi_viewer(self):
lc_plugin = self.imviz.plugins['Orientation']
lc_plugin.link_type = 'WCS'

# Should automatically be applied as reference to first viewer.
lc_plugin._obj.create_north_up_east_left(set_on_create=True)

# This would set a different reference to second viewer.
viewer_2 = self.imviz.create_image_viewer()
self.imviz.app.add_data_to_viewer("imviz-1", "has_wcs_1[SCI,1]")
lc_plugin.viewer = "imviz-1"
lc_plugin._obj.create_north_up_east_right(set_on_create=True)

assert self.viewer.state.reference_data.label == "North-up, East-left"
assert viewer_2.state.reference_data.label == "North-up, East-right"

# Both viewers should revert back to same reference when pixel-linked.
lc_plugin.link_type = 'Pixels'
assert self.viewer.state.reference_data.label == "has_wcs_1[SCI,1]"
assert viewer_2.state.reference_data.label == "has_wcs_1[SCI,1]"

lc_plugin.vue_reset_astrowidget_markers()
# FIXME: spacetelescope/jdaviz#2724 (remove AssertionError)
lc_plugin.link_type = 'WCS'
with pytest.raises(AssertionError):
assert self.viewer.state.reference_data.label == "Default orientation"
with pytest.raises(AssertionError):
assert viewer_2.state.reference_data.label == "Default orientation"

assert lc_plugin.need_clear_astrowidget_markers is False
lc_plugin.link_type.selected = 'WCS'
def test_custom_orientation(self):
lc_plugin = self.imviz.plugins['Orientation']
lc_plugin.link_type = 'WCS'
lc_plugin.viewer = "imviz-0"
lc_plugin.rotation_angle = 42 # Triggers auto-label
lc_plugin._obj.add_orientation(rotation_angle=None, east_left=True, label=None,
set_on_create=True, wrt_data=None)
assert self.viewer.state.reference_data.label == "CCW 42.00 deg (E-left)"
14 changes: 13 additions & 1 deletion jdaviz/configs/imviz/tests/test_wcs_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

from jdaviz.configs.imviz import wcs_utils
from jdaviz.configs.imviz.helper import base_wcs_layer_label
from jdaviz.configs.imviz.tests.utils import BaseImviz_WCS_GWCS
from jdaviz.configs.imviz.tests.utils import BaseImviz_WCS_GWCS, create_example_gwcs


def test_simple_fits_wcs():
Expand Down Expand Up @@ -161,3 +161,15 @@ def test_get_rotated_nddata_from_label_no_wcs(imviz_helper):
imviz_helper.load_data(a, data_label="no_wcs")
with pytest.raises(ValueError, match=r".*has no WCS for rotation"):
wcs_utils._get_rotated_nddata_from_label(imviz_helper.app, "no_wcs", 0 * u.deg)


def test_compute_scale():
# NOTE: compute_scale does not work with FITS WCS.
image_gwcs = create_example_gwcs((10, 10))
fiducial = [197.8925, -1.36555556] * u.deg

pixscale = wcs_utils.compute_scale(image_gwcs, fiducial, None)
assert_allclose(pixscale, 3.0555555555555554e-05)

pixscale = wcs_utils.compute_scale(image_gwcs, fiducial, None, pscale_ratio=1e+5)
assert_allclose(pixscale, 3.0555555555555554)
4 changes: 2 additions & 2 deletions jdaviz/configs/imviz/wcs_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ def compute_scale(wcs, fiducial, disp_axis, pscale_ratio=1):
"""
spectral = 'SPECTRAL' in wcs.output_frame.axes_type

if spectral and disp_axis is None:
if spectral and disp_axis is None: # pragma: no cover
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: This was copied from jwst so if spectral is probably important there but we do not care here.

raise ValueError('If input WCS is spectral, a disp_axis must be given')

crpix = np.array(wcs.invert(*fiducial))
Expand All @@ -588,7 +588,7 @@ def compute_scale(wcs, fiducial, disp_axis, pscale_ratio=1):
xscale *= pscale_ratio
yscale *= pscale_ratio

if spectral:
if spectral: # pragma: no cover
# Assuming scale doesn't change with wavelength
# Assuming disp_axis is consistent with DataModel.meta.wcsinfo.dispersion.direction
return yscale if disp_axis == 1 else xscale
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ astropy_header = true
doctest_plus = "enabled"
text_file_format = "rst"
addopts = "--doctest-rst --import-mode=append"
xfail_strict = true
filterwarnings = [
"error",
"ignore:numpy\\.ufunc size changed:RuntimeWarning",
Expand Down
Loading