Skip to content

Commit

Permalink
BUG: Fix spectrum and spaxel resetting X limits.
Browse files Browse the repository at this point in the history
TST: Add regression test that fails on main without this patch.

FEAT: Add viewer.get_limits() method for convenience and refactor some code to use it. Also refactor some code to use existing set_limits.
  • Loading branch information
pllim committed Oct 23, 2024
1 parent b47c128 commit 8cc0474
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 64 deletions.
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ Cubeviz

- Add missing styling to API hints entry for aperture_method in the spectral extraction plugin. [#3231]

- Fixed "spectrum at spaxel" tool so it no longer resets spectral axis zoom. [#3249]

Imviz
^^^^^

Expand Down
20 changes: 12 additions & 8 deletions jdaviz/configs/cubeviz/plugins/tests/test_tools.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import pytest
import numpy as np
from echo import delay_callback
from numpy.testing import assert_allclose
from regions import RectanglePixelRegion


@pytest.mark.filterwarnings('ignore:No observer defined on WCS')
def test_spectrum_at_spaxel(cubeviz_helper, spectrum1d_cube_with_uncerts):
def test_spectrum_at_spaxel_no_alt(cubeviz_helper, spectrum1d_cube_with_uncerts):
cubeviz_helper.load_data(spectrum1d_cube_with_uncerts, data_label='test')

flux_viewer = cubeviz_helper.app.get_viewer("flux-viewer")
Expand All @@ -19,6 +18,8 @@ def test_spectrum_at_spaxel(cubeviz_helper, spectrum1d_cube_with_uncerts):
assert len(flux_viewer.native_marks) == 2
assert len(spectrum_viewer.data()) == 1

assert_allclose(spectrum_viewer.get_limits(), (4.6228e-07, 4.6236e-07, 28, 92))

# Move to spaxel location
flux_viewer.toolbar.active_tool.on_mouse_move(
{'event': 'mousemove', 'domain': {'x': x, 'y': y}, 'altKey': False})
Expand All @@ -31,6 +32,10 @@ def test_spectrum_at_spaxel(cubeviz_helper, spectrum1d_cube_with_uncerts):
assert len(flux_viewer.native_marks) == 3
assert len(spectrum_viewer.data()) == 2

assert_allclose(spectrum_viewer.get_limits(),
(4.6228e-07, 4.6236e-07, 4, 15.6)) # Zoomed to spaxel
spectrum_viewer.set_limits(x_min=4.623e-07, x_max=4.6232e-07) # Zoom in X

# Check that a new subset was created
subsets = cubeviz_helper.app.get_subsets()
reg = subsets.get('Subset 1')[0]['region']
Expand All @@ -47,6 +52,9 @@ def test_spectrum_at_spaxel(cubeviz_helper, spectrum1d_cube_with_uncerts):
{'event': 'mouseleave', 'domain': {'x': x, 'y': y}, 'altKey': False})
assert flux_viewer.toolbar.active_tool._mark.visible is False

# Check that X is still zoomed but Y is reset.
assert_allclose(spectrum_viewer.get_limits(), (4.623e-07, 4.6232e-07, 28, 92))

# Deselect tool
flux_viewer.toolbar.active_tool = None
assert len(flux_viewer.native_marks) == 3
Expand Down Expand Up @@ -144,11 +152,7 @@ def test_spectrum_at_spaxel_altkey_true(cubeviz_helper, spectrum1d_cube,
t_linkedpan = flux_viewer.toolbar.tools['jdaviz:pixelpanzoommatch']
t_linkedpan.activate()
# TODO: When Cubeviz uses Astrowidgets, can just use center_on() for this part.
with delay_callback(flux_viewer.state, 'x_min', 'x_max', 'y_min', 'y_max'):
flux_viewer.state.x_min = 20
flux_viewer.state.y_min = 15
flux_viewer.state.x_max = 40
flux_viewer.state.y_max = 35
flux_viewer.set_limits(x_min=20, x_max=40, y_min=15, y_max=35)
v = uncert_viewer
assert v.state.zoom_center_x == flux_viewer.state.zoom_center_x
assert v.state.zoom_center_y == flux_viewer.state.zoom_center_y
Expand Down
6 changes: 3 additions & 3 deletions jdaviz/configs/cubeviz/plugins/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,11 @@ def on_mouse_move(self, data):
self._reset_profile_viewer_bounds()
self._mark.visible = False
else:
y_values = spectrum.flux[x, y, :]
y_values = spectrum.flux[x, y, :].value
if np.all(np.isnan(y_values)):
self._mark.visible = False
return
self._mark.update_xy(spectrum.spectral_axis.value, y_values)
self._mark.visible = True
self._profile_viewer.state.y_max = np.nanmax(y_values.value) * 1.2
self._profile_viewer.state.y_min = np.nanmin(y_values.value) * 0.8
self._profile_viewer.set_limits(
y_min=np.nanmin(y_values) * 0.8, y_max=np.nanmax(y_values) * 1.2)
12 changes: 4 additions & 8 deletions jdaviz/configs/default/plugins/tools.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from glue_jupyter.bqplot.profile import BqplotProfileView

from jdaviz.core.tools import SinglePixelRegion
from jdaviz.core.marks import PluginLine


__all__ = ['ProfileFromCube']


Expand All @@ -16,11 +16,8 @@ def __init__(self, *args, **kwargs):
self._data = None

def _reset_profile_viewer_bounds(self):
pv_state = self._profile_viewer.state
pv_state.x_min = self._previous_bounds[0]
pv_state.x_max = self._previous_bounds[1]
pv_state.y_min = self._previous_bounds[2]
pv_state.y_max = self._previous_bounds[3]
self._profile_viewer.set_limits(
y_min=self._previous_bounds[2], y_max=self._previous_bounds[3])

def activate(self):
self.viewer.add_event_callback(self.on_mouse_move, events=['mousemove', 'mouseleave'])
Expand All @@ -34,8 +31,7 @@ def activate(self):
self._mark = PluginLine(self._profile_viewer, visible=False)
self._profile_viewer.figure.marks = self._profile_viewer.figure.marks + [self._mark, ]
# Store these so we can revert to previous user-set zoom after preview view
pv_state = self._profile_viewer.state
self._previous_bounds = [pv_state.x_min, pv_state.x_max, pv_state.y_min, pv_state.y_max]
self._previous_bounds = self._profile_viewer.get_limits()
super().activate()

def deactivate(self):
Expand Down
11 changes: 11 additions & 0 deletions jdaviz/configs/default/plugins/viewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,17 @@ def set_limits(self, x_min=None, x_max=None, y_min=None, y_max=None):
if y_max is not None:
self.state.y_max = y_max

def get_limits(self):
"""Return current viewer axes limits.
Returns
-------
x_min, x_max, y_min, y_max : float
Lower/upper X/Y limits, respectively.
"""
return self.state.x_min, self.state.x_max, self.state.y_min, self.state.y_max

@property
def native_marks(self):
"""
Expand Down
11 changes: 2 additions & 9 deletions jdaviz/configs/imviz/plugins/catalogs/catalogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

from jdaviz.core.template_mixin import TableMixin
from jdaviz.core.user_api import PluginUserApi
from echo import delay_callback


__all__ = ['Catalogs']

Expand Down Expand Up @@ -307,13 +305,8 @@ def vue_zoom_in(self, *args, **kwargs):
y_min = min(y) - 50
y_max = max(y) + 50

imview = self.app._jdaviz_helper._default_viewer

with delay_callback(imview.state, 'x_min', 'x_max', 'y_min', 'y_max'):
imview.state.x_min = x_min
imview.state.x_max = x_max
imview.state.y_min = y_min
imview.state.y_max = y_max
self.app._jdaviz_helper._default_viewer.set_limits(
x_min=x_min, x_max=x_max, y_min=y_min, y_max=y_max)

return (x_min, x_max), (y_min, y_max)

Expand Down
30 changes: 15 additions & 15 deletions jdaviz/configs/imviz/tests/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ def test_panzoom_tools(self):
t = v.toolbar.tools['jdaviz:boxzoommatch']
# original limits (x_min, x_max, y_min, y_max): -0.5 9.5 -0.5 9.5
# original limits (zoom_center_x, zoom_center_y, zoom_radius): 4.5 4.5 5.0
original_limits = (v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max)
original_limits = v.get_limits()
assert_allclose(original_limits, (-0.5, 9.5, -0.5, 9.5))
original_center_rad = (v.state.zoom_center_x, v.state.zoom_center_y, v.state.zoom_radius)
assert_allclose(original_center_rad, (4.5, 4.5, 5.0))
assert_allclose((v2.state.x_min, v2.state.x_max, v2.state.y_min, v2.state.y_max), original_limits) # noqa
assert_allclose(v2.get_limits(), original_limits)
assert_allclose((v2.state.zoom_center_x, v2.state.zoom_center_y, v2.state.zoom_radius), original_center_rad) # noqa
t.activate()
t.save_prev_zoom()
v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max = (1, 8, 1, 8)
v.set_limits(x_min=1, x_max=8, y_min=1, y_max=8)
# second viewer should match these changes, wrt zoom center and radius
assert v2.state.zoom_center_x == v.state.zoom_center_x
assert v2.state.zoom_center_y == v.state.zoom_center_y
Expand All @@ -37,39 +37,39 @@ def test_panzoom_tools(self):

v.toolbar.tools['jdaviz:prevzoom'].activate()
# both should revert since they're still linked
assert (v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max) == (1, 8, 1, 8)
assert (v2.state.x_min, v2.state.x_max, v2.state.y_min, v2.state.y_max) == (1, 8, 1, 8)
assert v.get_limits() == (1, 8, 1, 8)
assert v2.get_limits() == (1, 8, 1, 8)

v.toolbar.tools['jdaviz:boxzoommatch'].deactivate()
v.toolbar.tools['jdaviz:homezoom'].activate()
assert_allclose((v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max), original_limits) # noqa
assert_allclose((v2.state.x_min, v2.state.x_max, v2.state.y_min, v2.state.y_max), (1, 8, 1, 8)) # noqa
assert_allclose(v.get_limits(), original_limits)
assert_allclose(v2.get_limits(), (1, 8, 1, 8))
v.toolbar.tools['jdaviz:prevzoom'].activate()
assert_allclose((v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max), (1, 8, 1, 8))
assert_allclose((v2.state.x_min, v2.state.x_max, v2.state.y_min, v2.state.y_max), (1, 8, 1, 8)) # noqa
assert_allclose(v.get_limits(), (1, 8, 1, 8))
assert_allclose(v2.get_limits(), (1, 8, 1, 8))
t.deactivate()

t_linkedpan = v.toolbar.tools['jdaviz:panzoommatch']
t_linkedpan.activate()
v.center_on((0, 0))
# make sure both viewers moved to the new center
assert_allclose((v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max), (-3.5, 3.5, -3.5, 3.5)) # noqa
assert_allclose((v2.state.x_min, v2.state.x_max, v2.state.y_min, v2.state.y_max), (-3.5, 3.5, -3.5, 3.5)) # noqa
assert_allclose(v.get_limits(), (-3.5, 3.5, -3.5, 3.5)) # noqa
assert_allclose(v2.get_limits(), (-3.5, 3.5, -3.5, 3.5)) # noqa
t_linkedpan.deactivate()

t_normpan = v.toolbar.tools['jdaviz:imagepanzoom']
t_normpan.activate()
t_normpan.on_click({'event': 'click', 'domain': {'x': 1, 'y': 1}})
# make sure only first viewer re-centered since this mode is not linked mode
assert_allclose((v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max), (-2.5, 4.5, -2.5, 4.5)) # noqa
assert_allclose((v2.state.x_min, v2.state.x_max, v2.state.y_min, v2.state.y_max), (-3.5, 3.5, -3.5, 3.5)) # noqa
assert_allclose(v.get_limits(), (-2.5, 4.5, -2.5, 4.5)) # noqa
assert_allclose(v2.get_limits(), (-3.5, 3.5, -3.5, 3.5)) # noqa
t_normpan.deactivate()

t_linkedpan.activate()
t_linkedpan.on_click({'event': 'click', 'domain': {'x': 2, 'y': 2}})
# make sure both viewers moved to the new center
assert_allclose((v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max), (-1.5, 5.5, -1.5, 5.5)) # noqa
assert_allclose((v2.state.x_min, v2.state.x_max, v2.state.y_min, v2.state.y_max), (-1.5, 5.5, -1.5, 5.5)) # noqa
assert_allclose(v.get_limits(), (-1.5, 5.5, -1.5, 5.5)) # noqa
assert_allclose(v2.get_limits(), (-1.5, 5.5, -1.5, 5.5)) # noqa
t_linkedpan.deactivate()


Expand Down
9 changes: 3 additions & 6 deletions jdaviz/configs/mosviz/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from astropy import units as u
from astropy.coordinates import SkyCoord
from astropy.table import QTable
from echo import delay_callback
from glue.core.data import Data
from glue.core.exceptions import IncompatibleAttribute

Expand Down Expand Up @@ -183,11 +182,9 @@ def _handle_image_zoom(self, msg):
cur_xcen = (imview.state.x_min + imview.state.x_max) * 0.5
cur_ycen = (imview.state.y_min + imview.state.y_max) * 0.5

with delay_callback(imview.state, 'x_min', 'x_max', 'y_min', 'y_max'):
imview.state.x_min = cur_xcen - x_height
imview.state.y_min = cur_ycen - height
imview.state.x_max = cur_xcen + x_height
imview.state.y_max = cur_ycen + height
imview.set_limits(
x_min=cur_xcen - x_height, x_max=cur_xcen + x_height,
y_min=cur_ycen - height, y_max=cur_ycen + height)

if center is not None:
imview.center_on(center)
Expand Down
20 changes: 10 additions & 10 deletions jdaviz/configs/mosviz/tests/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,28 @@ def test_viewer_axis_link(mosviz_helper, mos_spectrum1d, mos_spectrum2d):
s2dv = mosviz_helper.app.get_viewer('spectrum-2d-viewer')
sv = mosviz_helper.app.get_viewer('spectrum-viewer')

def _viewer_limits(v):
return (v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max)

t = sv.toolbar.tools['mosviz:boxzoom']
s2dv_orig_limits = _viewer_limits(s2dv)
s2dv_orig_limits = s2dv.get_limits()
assert_allclose(s2dv_orig_limits, (-0.5, 14.5, -0.5, 1023.5))

sv_orig_limits = _viewer_limits(sv)
assert_allclose(sv_orig_limits, (1e-06, 1.4999999999999999e-05, -1.9132802166577978, 1.5792128295073915)) # noqa
sv_orig_limits = sv.get_limits()
assert_allclose(sv_orig_limits, (1e-06, 1.4999999999999999e-05,
-1.9132802166577978, 1.5792128295073915))

t.activate()
# changes to sv should map to s2dv
sv.state.x_min = 1e-5
assert_allclose(_viewer_limits(sv), (1e-05, 1.4999999999999999e-05, -1.9132802166577978, 1.5792128295073915)) # noqa
assert_allclose(sv.get_limits(), (1e-05, 1.4999999999999999e-05,
-1.9132802166577978, 1.5792128295073915))
# shift in x_max caused by original padding
assert_allclose(_viewer_limits(s2dv), (9.000000000000002, 14.0, -0.5, 1023.5))
assert_allclose(s2dv.get_limits(), (9.000000000000002, 14.0, -0.5, 1023.5))

t2 = s2dv.toolbar.tools['mosviz:panzoom']
t2.activate()
# should have deactivated the tool in the spectrum-viewer
assert sv.toolbar.active_tool_id is None
# and now changes to s2dv should map to sv
s2dv.state.x_min = -0.5
assert_allclose(_viewer_limits(s2dv), (-0.5, 14.0, -0.5, 1023.5))
assert_allclose(_viewer_limits(sv), (5.000000000000006e-07, 1.4999999999999999e-05, -1.9132802166577978, 1.5792128295073915)) # noqa
assert_allclose(s2dv.get_limits(), (-0.5, 14.0, -0.5, 1023.5))
assert_allclose(sv.get_limits(), (5.000000000000006e-07, 1.4999999999999999e-05,
-1.9132802166577978, 1.5792128295073915))
8 changes: 3 additions & 5 deletions jdaviz/core/astrowidgets_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,9 @@ def center_on(self, point):
width = self.state.x_max - self.state.x_min
height = self.state.y_max - self.state.y_min

with delay_callback(self.state, 'x_min', 'x_max', 'y_min', 'y_max'):
self.state.x_min = pix[0] - (width * 0.5)
self.state.y_min = pix[1] - (height * 0.5)
self.state.x_max = self.state.x_min + width
self.state.y_max = self.state.y_min + height
self.set_limits(
x_min=pix[0] - (width * 0.5), x_max=self.state.x_min + width,
y_min=pix[1] - (height * 0.5), y_max=self.state.y_min + height)

def offset_by(self, dx, dy):
"""Move the center to a point that is given offset
Expand Down

0 comments on commit 8cc0474

Please # to comment.