BUG: Fix spectrum and spaxel resetting limits; FEAT: Add viewer.get_l…

…imits() (#3249)

* BUG: Fix spectrum and spaxel resetting X limits.

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.

* Fix cubeviz error and add test
that would fail without patching Y zoom also

Also keep Y zoom

* Move _is_moving

because we gotta move it, move it.

CHANGES.rst

@@ -77,6 +77,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
 ^^^^^
 

jdaviz/configs/cubeviz/plugins/tests/test_tools.py

@@ -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")
@@ -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})
@@ -31,6 +32,11 @@ 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, y_min=42, y_max=88)  # Zoom in X and Y
+
     # Check that a new subset was created
     subsets = cubeviz_helper.app.get_subsets()
     reg = subsets.get('Subset 1')[0]['region']
@@ -47,6 +53,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 zoom in both X and Y is kept.
+    assert_allclose(spectrum_viewer.get_limits(), (4.623e-07, 4.6232e-07, 42, 88))
+
     # Deselect tool
     flux_viewer.toolbar.active_tool = None
     assert len(flux_viewer.native_marks) == 3
@@ -144,11 +153,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

jdaviz/configs/cubeviz/plugins/tools.py

@@ -91,15 +91,40 @@ class SpectrumPerSpaxel(ProfileFromCube):
     action_text = 'See spectrum at a single spaxel'
     tool_tip = 'Click on the viewer and see the spectrum at that spaxel in the spectrum viewer'
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._is_moving = False
+
+    def activate(self):
+        super().activate()
+        for k in ("y_min", "y_max"):
+            self._profile_viewer.state.add_callback(k, self.on_limits_change)
+
+    def deactivate(self):
+        for k in ("y_min", "y_max"):
+            self._profile_viewer.state.remove_callback(k, self.on_limits_change)
+        super().deactivate()
+
+    def on_limits_change(self, *args):
+        if not self._is_moving:
+            self._previous_bounds = self._profile_viewer.get_limits()
+
     def on_mouse_move(self, data):
         if data['event'] == 'mouseleave':
             self._mark.visible = False
             self._reset_profile_viewer_bounds()
+            self._is_moving = False
             return
 
-        x = int(np.round(data['domain']['x']))
-        y = int(np.round(data['domain']['y']))
+        self._is_moving = True
+        try:
+            x = int(np.round(data['domain']['x']))
+            y = int(np.round(data['domain']['y']))
+            self._mouse_move_worker(x, y)
+        finally:
+            self._is_moving = False
 
+    def _mouse_move_worker(self, x, y):
         # Use the selected layer from coords_info as long as it's 3D
         coords_dataset = self.viewer.session.application._tools['g-coords-info'].dataset.selected
         if coords_dataset == 'auto':
@@ -134,11 +159,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)

jdaviz/configs/default/plugins/tools.py

@@ -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']
 
 
@@ -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'])
@@ -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):

jdaviz/configs/default/plugins/viewers.py

@@ -153,8 +153,9 @@ def set_limits(self, x_min=None, x_max=None, y_min=None, y_max=None):
             upper-limit of y-axis (in current axes units)
         """
         for val in (x_min, x_max, y_min, y_max):
-            if val is not None and not isinstance(val, (float, int)):
-                raise TypeError('all arguments must be None, int, or float')
+            if val is not None and not isinstance(val, (float, int, np.float32)):
+                raise TypeError('All arguments must be None, int, or float, '
+                                f'but got: {type(val)}')
 
         with delay_callback(self.state, 'x_min', 'x_max', 'y_min', 'y_max'):
             if x_min is not None:
@@ -166,6 +167,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):
         """

jdaviz/configs/imviz/plugins/catalogs/catalogs.py

@@ -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']
 
@@ -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)
 

jdaviz/configs/imviz/tests/test_astrowidgets_api.py

@@ -30,47 +30,34 @@ class TestCenterOffset(BaseImviz_WCS_NoWCS):
 
     def test_center_offset_pixel(self):
         self.viewer.center_on((0, 1))
-        assert_allclose((self.viewer.state.x_min, self.viewer.state.x_max,
-                         self.viewer.state.y_min, self.viewer.state.y_max),
-                        (-5, 5, -4, 6))
+        assert_allclose(self.viewer.get_limits(), (-5, 5, -4, 6))
 
         self.viewer.offset_by(1 * u.pix, -1 * u.dimensionless_unscaled)
-        assert_allclose((self.viewer.state.x_min, self.viewer.state.x_max,
-                         self.viewer.state.y_min, self.viewer.state.y_max),
-                        (-4, 6, -5, 5))
+        assert_allclose(self.viewer.get_limits(), (-4, 6, -5, 5))
 
         self.viewer.offset_by(1, 0)
-        assert_allclose((self.viewer.state.x_min, self.viewer.state.x_max,
-                         self.viewer.state.y_min, self.viewer.state.y_max),
-                        (-3, 7, -5, 5))
+        assert_allclose(self.viewer.get_limits(), (-3, 7, -5, 5))
 
         # Out-of-bounds centering is now allowed because it is needed
         # for dithering use case.
         self.viewer.center_on((-1, 99999))
-        assert_allclose((self.viewer.state.x_min, self.viewer.state.x_max,
-                         self.viewer.state.y_min, self.viewer.state.y_max),
-                        (-6, 4, 9.99940e+04, 1.00004e+05))
+        assert_allclose(self.viewer.get_limits(), (-6, 4, 9.99940e+04, 1.00004e+05))
 
         # Sometimes invalid WCS also gives such output, should be no-op
         self.viewer.center_on((np.array(np.nan), np.array(np.nan)))
-        assert_allclose((self.viewer.state.x_min, self.viewer.state.x_max,
-                         self.viewer.state.y_min, self.viewer.state.y_max),
-                        (-6, 4, 9.99940e+04, 1.00004e+05))
+        assert_allclose(self.viewer.get_limits(), (-6, 4, 9.99940e+04, 1.00004e+05))
 
     def test_center_offset_sky(self):
         # Blink to the one with WCS because the last loaded data is shown.
         self.viewer.blink_once()
 
         sky = self.wcs.pixel_to_world(0, 1)
         self.viewer.center_on(sky)
-        assert_allclose((self.viewer.state.x_min, self.viewer.state.x_max,
-                         self.viewer.state.y_min, self.viewer.state.y_max),
-                        (-5, 5, -4, 6))
+        assert_allclose(self.viewer.get_limits(), (-5, 5, -4, 6))
 
         dsky = 0.1 * u.arcsec
         self.viewer.offset_by(-dsky, dsky)
-        assert_allclose((self.viewer.state.x_min, self.viewer.state.x_max,
-                         self.viewer.state.y_min, self.viewer.state.y_max),
+        assert_allclose(self.viewer.get_limits(),
                         (-4.9, 5.1, -3.90000000002971, 6.09999999997029))
 
         # Cannot mix pixel with sky
@@ -102,24 +89,18 @@ def test_center_on_pix(self):
         self.viewer.center_on((0, 0))
         expected_position = [-6.75, 4.25, -5.75, 5.25]
         rtol = 1e-4
-        assert_allclose((self.viewer.state.x_min, self.viewer.state.x_max,
-                         self.viewer.state.y_min, self.viewer.state.y_max),
-                        expected_position, rtol=rtol)
+        assert_allclose(self.viewer.get_limits(), expected_position, rtol=rtol)
 
         # This is the first data.
         self.viewer.blink_once()
         self.viewer.center_on((0, 0))
-        assert_allclose((self.viewer.state.x_min, self.viewer.state.x_max,
-                         self.viewer.state.y_min, self.viewer.state.y_max),
-                        [-5.75, 5.25, -5.75, 5.25], rtol=rtol)
+        assert_allclose(self.viewer.get_limits(), [-5.75, 5.25, -5.75, 5.25], rtol=rtol)
 
         # Centering by sky on second data.
         self.viewer.blink_once()
         sky = self.wcs_2.pixel_to_world(0, 0)
         self.viewer.center_on(sky)
-        assert_allclose((self.viewer.state.x_min, self.viewer.state.x_max,
-                         self.viewer.state.y_min, self.viewer.state.y_max),
-                        expected_position, rtol=rtol)
+        assert_allclose(self.viewer.get_limits(), expected_position, rtol=rtol)
 
 
 class TestZoom(BaseImviz_WCS_NoWCS):
@@ -135,8 +116,7 @@ def test_invalid_zoom(self):
 
     def assert_zoom_results(self, zoom_level, x_min, x_max, y_min, y_max, dpix):
         assert_allclose(self.viewer.zoom_level, zoom_level)
-        assert_allclose((self.viewer.state.x_min, self.viewer.state.x_max,
-                         self.viewer.state.y_min, self.viewer.state.y_max),
+        assert_allclose(self.viewer.get_limits(),
                         (x_min + dpix, x_max + dpix,
                          y_min + dpix, y_max + dpix))
 

jdaviz/configs/imviz/tests/test_tools.py

@@ -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
@@ -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))
+        assert_allclose(v2.get_limits(), (-3.5, 3.5, -3.5, 3.5))
         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))
+        assert_allclose(v2.get_limits(), (-3.5, 3.5, -3.5, 3.5))
         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))
+        assert_allclose(v2.get_limits(), (-1.5, 5.5, -1.5, 5.5))
         t_linkedpan.deactivate()