Account for all layers in reset limits/zoom home (#1897)

* override reset_x_limits for profile viewer
* custom pixel or WCS implementation for image viewer reset_limits
* pixel-fallback if wcs bounds fails
* update tests to use allclose
* new viewer to adopt correct reset_limits behavior based on linking

CHANGES.rst

@@ -11,6 +11,9 @@ New Features
 - Model fitting plugin can optionally expose the residuals as an additional data collection entry.
   [#1864, #1891]
 
+- Resetting viewer limits (via ``reset_limits`` or the zoom home button) now accounts for all visible
+  data layers instead of just the reference data. [#1897]
+
 Cubeviz
 ^^^^^^^
 

jdaviz/app.py

@@ -1656,6 +1656,17 @@ def _on_new_viewer(self, msg, vid=None, name=None):
             msg.cls, data=msg.data, show=False)
         viewer.figure_widget.layout.height = '100%'
 
+        links_control_plugin = self._jdaviz_helper.plugins.get('Links Control', None)
+        if links_control_plugin is not None:
+            viewer.state.linked_by_wcs = links_control_plugin.link_type.selected == 'WCS'
+        elif len(self._viewer_store):
+            # The plugin would only not exist for instances of Imviz where the user has
+            # intentionally removed the Links Control plugin, but in that case we will
+            # adopt "linked_by_wcs" from the first (assuming all are the same)
+            # NOTE: deleting the default viewer is forbidden both by API and UI, but if
+            # for some reason that was the case here, linked_by_wcs will default to False
+            viewer.state.linked_by_wcs = list(self._viewer_store.values())[0].state.linked_by_wcs
+
         if msg.x_attr is not None:
             x = msg.data.id[msg.x_attr]
             viewer.state.x_att = x

jdaviz/app.vue

@@ -313,6 +313,18 @@ div.output_wrapper {
   display: none;
 }
 
+.imviz .lm_close {
+  /* hide the close button on the right to prevent closing the default viewer
+     since we cannot easily discriminate between different viewers in the filter here */
+  display: none !important;
+}
+
+.imviz .lm_tab[title="imviz-0"] > .lm_close_tab {
+  /* hide the close button on the tab for imviz-0 only to
+     prevent closing the default viewer */
+  display: none;
+}
+
 .v-toolbar__items .v-btn {
   /* allow v-toolbar-items styling to pass through tooltip wrapping span */
   /* css is copied from .v-toolbar__items>.v-btn */

jdaviz/configs/imviz/helper.py

@@ -620,3 +620,7 @@ def link_image_data(app, link_type='pixels', wcs_fallback_scheme='pixels', wcs_u
                                              sender=app))
         # reset the progress spinner
         link_plugin.linking_in_progress = False
+
+    for viewer in app._viewer_store.values():
+        # viewer-state needs to know link type for reset_limits behavior
+        viewer.state.linked_by_wcs = link_type == 'wcs'

jdaviz/configs/imviz/plugins/viewers.py

@@ -10,6 +10,7 @@
 from jdaviz.core.events import SnackbarMessage
 from jdaviz.core.helpers import data_has_valid_wcs
 from jdaviz.core.registries import viewer_registry
+from jdaviz.core.freezable_state import FreezableBqplotImageViewerState
 from jdaviz.configs.default.plugins.viewers import JdavizViewerMixin
 
 __all__ = ['ImvizImageView']
@@ -29,6 +30,7 @@ class ImvizImageView(JdavizViewerMixin, BqplotImageView, AstrowidgetsImageViewer
                 ]
 
     default_class = None
+    _state_cls = FreezableBqplotImageViewerState
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)

jdaviz/configs/imviz/tests/test_linking.py

@@ -22,6 +22,13 @@ def test_pixel_linking(self):
         self.imviz.link_data(link_type='pixels', error_on_fail=True)
         self.check_all_pixel_links()
 
+    @property
+    def default_viewer_limits(self):
+        return (self.imviz.default_viewer.state.x_min,
+                self.imviz.default_viewer.state.x_max,
+                self.imviz.default_viewer.state.y_min,
+                self.imviz.default_viewer.state.y_max)
+
 
 class TestLink_WCS_NoWCS(BaseImviz_WCS_NoWCS, BaseLinkHandler):
 
@@ -88,13 +95,21 @@ def test_badwcs_no_crash(self):
 class TestLink_WCS_WCS(BaseImviz_WCS_WCS, BaseLinkHandler):
 
     def test_wcslink_affine_with_extras(self):
+        orig_pixel_limits = self.default_viewer_limits
+        assert_allclose(orig_pixel_limits, (-0.5, 9.5, -0.5, 9.5))
+
         self.imviz.link_data(link_type='wcs', wcs_fallback_scheme=None, error_on_fail=True)
         links = self.imviz.app.data_collection.external_links
         assert len(links) == 1
         assert isinstance(links[0], OffsetLink)
 
         assert self.viewer.get_link_type('has_wcs_2[SCI,1]') == 'wcs'
 
+        # linking should not change axes limits, but should when resetting
+        assert_allclose(self.default_viewer_limits, orig_pixel_limits)
+        self.imviz.default_viewer.state.reset_limits()
+        assert_allclose(self.default_viewer_limits, (-1.5, 9.5, -1, 10))
+
         # Customize display on second image (last loaded).
         self.viewer.set_colormap('Viridis')
         self.viewer.stretch = 'sqrt'

jdaviz/configs/imviz/tests/test_tools.py

@@ -1,4 +1,5 @@
 import numpy as np
+from numpy.testing import assert_allclose
 from regions import RectanglePixelRegion
 
 from jdaviz.configs.imviz.tests.utils import BaseImviz_WCS_WCS
@@ -12,6 +13,8 @@ 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 = (v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max)
+        assert_allclose(original_limits, (-0.5, 9.5, -0.5, 9.5))
         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)
@@ -20,8 +23,8 @@ 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) == (-0.5, 9.5, -0.5, 9.5)  # noqa
-        assert (v2.state.x_min, v2.state.x_max, v2.state.y_min, v2.state.y_max) == (-0.5, 9.5, -0.5, 9.5)  # noqa
+        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), original_limits)  # noqa
 
         v.toolbar.tools['jdaviz:prevzoom'].activate()
         # both should revert since they're still linked
@@ -30,34 +33,34 @@ def test_panzoom_tools(self):
 
         v.toolbar.tools['jdaviz:boxzoommatch'].deactivate()
         v.toolbar.tools['jdaviz:homezoom'].activate()
-        assert (v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max) == (-0.5, 9.5, -0.5, 9.5)  # noqa
-        assert (v2.state.x_min, v2.state.x_max, v2.state.y_min, v2.state.y_max) == (1, 8, 1, 8)
+        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
         v.toolbar.tools['jdaviz:prevzoom'].activate()
-        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_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
         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 (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 (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.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
         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 (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 (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.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
         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 (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 (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.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
         t_linkedpan.deactivate()
 
 

jdaviz/core/freezable_state.py

@@ -1,3 +1,6 @@
+from echo import delay_callback
+import numpy as np
+
 from glue.viewers.profile.state import ProfileViewerState
 from glue_jupyter.bqplot.image.state import BqplotImageViewerState
 from glue.viewers.matplotlib.state import DeferredDrawCallbackProperty as DDCProperty
@@ -19,6 +22,84 @@ def __setattr__(self, k, v):
 class FreezableProfileViewerState(ProfileViewerState, FreezableState):
     show_uncertainty = DDCProperty(False, docstring='Whether to show data uncertainties')
 
+    def _reset_x_limits(self, *event):
+        # override glue's _reset_x_limits to account for all layers,
+        # not just reference data (_reset_y_limits already does so)
+        # This is essentially copied directly from Glue's
+        # ProfileViewerState._reset_y_limits but modified for x-limits
+        if self.reference_data is None or self.x_att_pixel is None:
+            return
+
+        x_min, x_max = np.inf, -np.inf
+        for layer in self.layers:
+            try:
+                profile = layer.profile
+            except Exception:  # nosec
+                # e.g. incompatible subset
+                continue
+            if profile is not None:
+                x, y = profile
+                if len(x) > 0:
+                    x_min = min(x_min, np.nanmin(x))
+                    x_max = max(x_max, np.nanmax(x))
+
+        with delay_callback(self, 'x_min', 'x_max'):
+            self.x_min = x_min
+            self.x_max = x_max
+
 
 class FreezableBqplotImageViewerState(BqplotImageViewerState, FreezableState):
-    pass
+    linked_by_wcs = False
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+    def reset_limits(self, *event):
+        wcs_success = False
+        if self.linked_by_wcs and self.reference_data.coords is not None:
+            x_min, x_max = np.inf, -np.inf
+            y_min, y_max = np.inf, -np.inf
+
+            for layer in self.layers:
+                if not layer.visible:
+                    continue
+
+                data = next((x for x in self.data_collection if x.label == layer.layer.data.label))
+                if data.coords is None:
+                    # if no layers have coords, then wcs_success will remain
+                    # false and limits will fallback based on pixel limit
+                    continue
+
+                pixel_ids = layer.layer.pixel_component_ids
+                world_bottom_left = data.coords.pixel_to_world(0, 0)
+                world_top_right = data.coords.pixel_to_world(layer.layer.data[pixel_ids[1]].max(),
+                                                             layer.layer.data[pixel_ids[0]].max())
+
+                pixel_bottom_left = self.reference_data.coords.world_to_pixel(world_bottom_left)
+                pixel_top_right = self.reference_data.coords.world_to_pixel(world_top_right)
+
+                x_min = min(x_min, pixel_bottom_left[0] - 0.5)
+                x_max = max(x_max, pixel_top_right[0] + 0.5)
+                y_min = min(y_min, pixel_bottom_left[1] - 0.5)
+                y_max = max(y_max, pixel_top_right[1] + 0.5)
+                wcs_success = True
+
+        if not wcs_success:
+            x_min, x_max = -0.5, -np.inf
+            y_min, y_max = -0.5, -np.inf
+            for layer in self.layers:
+                if not layer.visible:
+                    continue
+                pixel_ids = layer.layer.pixel_component_ids
+
+                x_max = max(x_max, layer.layer.data[pixel_ids[1]].max() + 0.5)
+                y_max = max(y_max, layer.layer.data[pixel_ids[0]].max() + 0.5)
+
+        with delay_callback(self, 'x_min', 'x_max', 'y_min', 'y_max'):
+            self.x_min = x_min
+            self.x_max = x_max
+            self.y_min = y_min
+            self.y_max = y_max
+            # We need to adjust the limits in here to avoid triggering all
+            # the update events then changing the limits again.
+            self._adjust_limits_aspect()