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

BUG: Fix click to center on multiple visible layers when linked by WCS #2750

Merged
merged 2 commits into from
Mar 13, 2024
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
10 changes: 7 additions & 3 deletions jdaviz/configs/imviz/plugins/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from glue_jupyter.utils import debounced

from jdaviz.core.tools import BoxZoom, PanZoom, _MatchedZoomMixin
from jdaviz.configs.imviz.helper import get_top_layer_index

__all__ = []

Expand Down Expand Up @@ -37,15 +38,18 @@

def on_click(self, data):
# Find visible layers
visible_layers = [layer for layer in self.viewer.state.layers if layer.visible]
if len(visible_layers) == 0:
try:
i = get_top_layer_index(self.viewer)
except IndexError:
return

Check warning on line 44 in jdaviz/configs/imviz/plugins/tools.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/imviz/plugins/tools.py#L43-L44

Added lines #L43 - L44 were not covered by tests
if i is None:
return

# Same data as mousemove event in Imviz viewer.
# Any other config that wants this functionality has to have the following:
# viewer._get_real_xy()
# viewer.center_on() --> inherited from AstrowidgetsImageViewerMixin
image = visible_layers[0].layer
image = self.viewer.state.layers[i].layer
x = data['domain']['x']
y = data['domain']['y']
if x is None or y is None: # Out of bounds
Expand Down
4 changes: 2 additions & 2 deletions jdaviz/configs/imviz/plugins/viewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@ def blink_once(self, reversed=False):
def on_limits_change(self, *args):
try:
i = get_top_layer_index(self)
if i is None:
return
except IndexError:
if self.compass is not None:
self.compass.clear_compass()
return
if i is None:
return
self.set_compass(self.state.layers[i].layer)

@property
Expand Down
45 changes: 44 additions & 1 deletion jdaviz/configs/imviz/tests/test_tools.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import numpy as np
import pytest
from astropy.coordinates import SkyCoord
from astropy.nddata import NDData
from numpy.testing import assert_allclose

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


class TestPanZoomTools(BaseImviz_WCS_WCS):
Expand Down Expand Up @@ -64,6 +67,46 @@ def test_panzoom_tools(self):
t_linkedpan.deactivate()


@pytest.mark.parametrize("link_type", ["Pixels", "WCS"])
def test_panzoom_click_center_linking(imviz_helper, link_type):
"""https://github.com/spacetelescope/jdaviz/issues/2749"""
v = imviz_helper.default_viewer._obj

# Since we are not really displaying, need this to test pan/zoom.
v.shape = (100, 100)
v.state._set_axes_aspect_ratio(1)

arr_big = np.ones((40, 30), dtype=int)
w_big = create_example_gwcs(arr_big.shape)
arr_small = np.ones((20, 15), dtype=int)
w_small = create_example_gwcs(arr_small.shape)

imviz_helper.load_data(NDData(arr_big, wcs=w_big), data_label="big")
imviz_helper.load_data(NDData(arr_small, wcs=w_small), data_label="small")

lc_plugin = imviz_helper.plugins['Orientation']
lc_plugin.link_type = link_type

coo = SkyCoord(ra=197.89262754541807, dec=-1.3644568140486624, unit="deg")

if link_type == "WCS":
mouseover_loc = v.state.reference_data.coords.world_to_pixel(coo)
else: # Pixels
mouseover_loc = w_small.world_to_pixel(coo)

t = v.toolbar.tools['jdaviz:imagepanzoom']
t.activate()
t.on_click({'event': 'click', 'domain': {'x': mouseover_loc[0], 'y': mouseover_loc[1]}})
t.deactivate()

# We want to make sure click centers viewer to where it is supposed to be.
cur_cen = v._get_center_skycoord()
v.center_on(coo)
real_cen = v._get_center_skycoord()
assert_allclose(cur_cen.ra.deg, real_cen.ra.deg)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this patch, it would fail with this error here:

E           AssertionError:
E           Not equal to tolerance rtol=1e-07, atol=0
E
E           Mismatched elements: 1 / 1 (100%)
E           Max absolute difference: 0.00015008
E           Max relative difference: 7.58381226e-07
E            x: array(197.892778)
E            y: array(197.892628)

assert_allclose(cur_cen.dec.deg, real_cen.dec.deg)


def test_blink(imviz_helper):
viewer = imviz_helper.default_viewer._obj

Expand Down