plugin is_active skip_if_no_updates_since_plugin_last_active decorator

jdaviz/configs/default/plugins/plot_options/plot_options.py

@@ -12,7 +12,8 @@
 
 from jdaviz.core.registries import tray_registry
 from jdaviz.core.template_mixin import (PluginTemplateMixin, ViewerSelect, LayerSelect,
-                                        PlotOptionsSyncState, Plot)
+                                        PlotOptionsSyncState, Plot,
+                                        skip_if_no_updates_since_last_active)
 from jdaviz.core.user_api import PluginUserApi
 from jdaviz.core.tools import ICON_DIR
 from jdaviz.core.custom_traitlets import IntHandleEmpty
@@ -254,12 +255,6 @@ class PlotOptions(PluginTemplateMixin):
     show_viewer_labels = Bool(True).tag(sync=True)
 
     def __init__(self, *args, **kwargs):
-        # track whether the stretch histogram needs an update (some entry has changed) if is_active
-        # becomes True, to address potential lag from a backlog of calls to
-        # _update_stretch_histogram if the browser throttles pings
-        # (https://github.com/spacetelescope/jdaviz/issues/2317)
-        self._stretch_histogram_needs_update = True
-
         super().__init__(*args, **kwargs)
         self.viewer = ViewerSelect(self, 'viewer_items', 'viewer_selected', 'multiselect')
         self.layer = LayerSelect(self, 'layer_items', 'layer_selected', 'viewer_selected', 'multiselect')  # noqa
@@ -504,6 +499,7 @@ def vue_set_value(self, data):
 
     @observe('is_active', 'layer_selected', 'viewer_selected',
              'stretch_hist_zoom_limits')
+    @skip_if_no_updates_since_last_active()
     def _update_stretch_histogram(self, msg={}):
         if not hasattr(self, 'viewer'):  # pragma: no cover
             # plugin hasn't been fully initialized yet
@@ -520,15 +516,6 @@ def _update_stretch_histogram(self, msg={}):
             # its type
             msg = {}
 
-        if msg.get('name', None) == 'is_active' and not self._stretch_histogram_needs_update:
-            # this could be re-triggering if the browser is throttling pings on the js-side
-            # and since this is expensive, could result in laggy behavior
-            return
-        elif msg.get('name', None) != 'is_active' and not self.is_active:
-            # next time is_active becomes True, we need to update the histogram plot
-            self._stretch_histogram_needs_update = True
-            return
-
         if not self.stretch_function_sync.get('in_subscribed_states'):  # pragma: no cover
             # no (image) viewer with stretch function options
             return
@@ -626,8 +613,6 @@ def _update_stretch_histogram(self, msg={}):
             # we'll force the traitlet to trigger a change
             hist_mark.send_state('sample')
 
-        self._stretch_histogram_needs_update = False
-
     @observe('stretch_vmin_value')
     def _stretch_vmin_changed(self, msg=None):
         self.stretch_histogram.marks['vmin'].x = [self.stretch_vmin_value, self.stretch_vmin_value]

jdaviz/configs/imviz/plugins/compass/compass.py

@@ -2,7 +2,8 @@
 
 from jdaviz.core.events import AddDataMessage, RemoveDataMessage, CanvasRotationChangedMessage
 from jdaviz.core.registries import tray_registry
-from jdaviz.core.template_mixin import PluginTemplateMixin, ViewerSelectMixin
+from jdaviz.core.template_mixin import (PluginTemplateMixin, ViewerSelectMixin,
+                                        skip_if_no_updates_since_last_active)
 from jdaviz.core.user_api import PluginUserApi
 
 __all__ = ['Compass']
@@ -60,7 +61,8 @@ def _set_compass_rotation(self):
         self.canvas_flip_horizontal = viewer_item.get('canvas_flip_horizontal', False)
 
     @observe("viewer_selected", "is_active")
-    def _compass_with_new_viewer(self, *args, **kwargs):
+    @skip_if_no_updates_since_last_active()
+    def _compass_with_new_viewer(self, msg={}):
         if not hasattr(self, 'viewer'):
             # mixin object not yet initialized
             return

jdaviz/configs/imviz/plugins/footprints/footprints.py

@@ -232,6 +232,8 @@ def _on_overlay_remove(self, lbl):
                                    if getattr(m, 'overlay', None) != lbl]
 
     @observe('is_active', 'viewer_items')
+    # NOTE: intentionally not using skip_if_no_updates_since_last_active since this only controls
+    # visibility of overlay (and creating the first instance)
     def _on_is_active_changed(self, *args):
         if not hasattr(self, 'overlay'):  # pragma: nocover
             # plugin/traitlet startup

jdaviz/configs/imviz/plugins/line_profile_xy/line_profile_xy.py

@@ -3,7 +3,8 @@
 from jdaviz.configs.imviz.helper import get_top_layer_index
 from jdaviz.core.events import ViewerAddedMessage
 from jdaviz.core.registries import tray_registry
-from jdaviz.core.template_mixin import PluginTemplateMixin, ViewerSelectMixin, Plot
+from jdaviz.core.template_mixin import (PluginTemplateMixin, ViewerSelectMixin, Plot,
+                                        skip_if_no_updates_since_last_active)
 
 __all__ = ['LineProfileXY']
 
@@ -51,7 +52,7 @@ def _on_viewer_added(self, msg):
         self._create_viewer_callbacks(self.app.get_viewer_by_id(msg.viewer_id))
 
     @observe('is_active')
-    def on_is_active_changed(self, *args):
+    def _is_active_changed(self, msg):
         # subscribe/unsubscribe to keypress events across all viewers
         for viewer in self.app._viewer_store.values():
             if not hasattr(viewer, 'figure'):
@@ -64,8 +65,9 @@ def on_is_active_changed(self, *args):
             else:
                 viewer.remove_event_callback(callback)
 
-        if self.is_active:
-            self.vue_draw_plot()
+        # pass along the msg object so that @skip_if_no_updates_since_last_active can be used
+        # to avoid re-drawing if no changes since the last time is_active was set
+        self.vue_draw_plot(msg)
 
     def _on_viewer_key_event(self, viewer, data):
         if data['key'] == 'l':
@@ -83,9 +85,10 @@ def _on_viewer_key_event(self, viewer, data):
             self.vue_draw_plot()
 
     @observe("viewer_selected")
-    def vue_draw_plot(self, *args, **kwargs):
+    @skip_if_no_updates_since_last_active()  # called with msg passed along from _is_active_changed
+    def vue_draw_plot(self, msg={}):
         """Draw line profile plots for given Data across given X and Y indices (0-indexed)."""
-        if not self.selected_x or not self.selected_y or not self.is_active:
+        if not self.selected_x or not self.selected_y:
             return
 
         viewer = self.viewer.selected_obj

jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py

@@ -30,7 +30,8 @@
                                         SpectralSubsetSelectMixin,
                                         DatasetSpectralSubsetValidMixin,
                                         SubsetSelect,
-                                        SPATIAL_DEFAULT_TEXT)
+                                        SPATIAL_DEFAULT_TEXT,
+                                        skip_if_no_updates_since_last_active)
 from jdaviz.core.user_api import PluginUserApi
 from jdaviz.core.tools import ICON_DIR
 
@@ -209,13 +210,11 @@ def _on_viewer_subsets_changed(self, msg):
         """
         if (msg.subset.label in [self.spectral_subset_selected,
                                  self.spatial_subset_selected,
-                                 self.continuum_subset_selected]
-                and self.is_active):
-            self._calculate_statistics()
+                                 self.continuum_subset_selected]):
+            self._calculate_statistics(msg)
 
     def _on_global_display_unit_changed(self, msg):
-        if self.is_active:
-            self._calculate_statistics()
+        self._calculate_statistics(msg)
 
     @observe('is_active')
     def _is_active_changed(self, msg):
@@ -224,8 +223,7 @@ def _is_active_changed(self, msg):
 
         for pos, mark in self.marks.items():
             mark.visible = self.is_active
-        if self.is_active:
-            self._calculate_statistics()
+        self._calculate_statistics(msg)
 
     @deprecated(since="3.6", alternative="keep_active")
     def show_continuum_marks(self):
@@ -302,7 +300,8 @@ def _on_identified_line_changed(self, msg):
 
     @observe("spatial_subset_selected", "spectral_subset_selected", "dataset_selected",
              "continuum_subset_selected", "width")
-    def _calculate_statistics(self, *args, **kwargs):
+    @skip_if_no_updates_since_last_active()
+    def _calculate_statistics(self, msg={}):
         """
         Run the line analysis functions on the selected data/subset and
         display the results.

jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py

@@ -8,7 +8,8 @@
 from jdaviz.core.template_mixin import (PluginTemplateMixin,
                                         SelectPluginComponent,
                                         DatasetSelect,
-                                        AddResults)
+                                        AddResults,
+                                        skip_if_no_updates_since_last_active)
 from jdaviz.core.user_api import PluginUserApi
 from jdaviz.core.custom_traitlets import IntHandleEmpty, FloatHandleEmpty
 from jdaviz.core.marks import PluginLine
@@ -199,7 +200,7 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
         self._marks = {}
-        self._do_marks = kwargs.get('interactive', True)
+        self._do_marks = False
 
         # TRACE
         self.trace_trace = DatasetSelect(self,
@@ -317,6 +318,10 @@ def __init__(self, *args, **kwargs):
         self.ext_add_results.label_whitelist_overwrite = ['Spectrum 1D']
         self.ext_results_label_default = 'Spectrum 1D'
 
+        # continue to not use live-preview marks for the instance of the plugin used for the
+        # initial spectral extraction during load_data
+        self._do_marks = kwargs.get('interactive', True)
+
     @property
     def _default_spectrum_viewer_reference_name(self):
         return self.app._jdaviz_helper._default_spectrum_viewer_reference_name
@@ -408,10 +413,16 @@ def update_marks(self, step=None):
             else:
                 raise ValueError("step must be one of: trace, bg, ext")
 
-    @observe('is_active', 'interactive_extract')
-    def _update_plugin_marks(self, *args):
+    # also listens to is_active from any _interaction_in_*_step methods
+    def _update_plugin_marks(self, msg={}):
         if not self._do_marks:
             return
+        if self.app._jdaviz_helper is None:
+            return
+        if not len(self._marks):
+            # plugin has never been opened, no need to create marks just to hide them,
+            # we'll create marks when the plugin is first opened
+            return
 
         if not (self.is_active):
             for step, mark in self.marks.items():
@@ -420,26 +431,9 @@ def _update_plugin_marks(self, *args):
 
         if self.active_step == '':
             # on load, default to 'extract' (this will then trigger the observe to update the marks)
-            self._interaction_in_ext_step()
+            self.active_step = 'ext'
             return
 
-        # update extracted 1d spectrum preview, regardless of the step
-        if self.interactive_extract:
-            try:
-                sp1d = self.export_extract_spectrum(add_data=False)
-            except Exception as e:
-                # NOTE: ignore error, but will be raised when clicking ANY of the export buttons
-                # NOTE: FitTrace or manual background are often giving a
-                # "background regions overlapped" error from specreduce
-                self.ext_specreduce_err = repr(e)
-                self.marks['extract'].clear()
-            else:
-                self.ext_specreduce_err = ''
-                self.marks['extract'].update_xy(sp1d.spectral_axis.value,
-                                                sp1d.flux.value)
-        else:
-            self.marks['extract'].clear()
-
         display_marks = {'trace': ['trace', 'extract'],
                          'bg': ['trace',
                                 'bg1_center', 'bg1_lower', 'bg1_upper',
@@ -462,6 +456,7 @@ def marks(self):
 
         if not self._do_marks:
             return {}
+
         viewer2d = self.app.get_viewer(self._default_spectrum_2d_viewer_reference_name)
         viewer1d = self.app.get_viewer(self._default_spectrum_viewer_reference_name)
 
@@ -494,14 +489,49 @@ def marks(self):
 
         return self._marks
 
-    @observe('trace_dataset_selected', 'trace_type_selected',
+    @observe('interactive_extract')
+    @skip_if_no_updates_since_last_active()
+    def _update_interactive_extract(self, event={}):
+        # also called by any of the _interaction_in_*_step
+        if not self._do_marks:
+            return False
+
+        if self.interactive_extract:
+            try:
+                sp1d = self.export_extract_spectrum(add_data=False)
+            except Exception as e:
+                # NOTE: ignore error, but will be raised when clicking ANY of the export buttons
+                # NOTE: FitTrace or manual background are often giving a
+                # "background regions overlapped" error from specreduce
+                self.ext_specreduce_err = repr(e)
+                self.marks['extract'].clear()
+            else:
+                self.ext_specreduce_err = ''
+                self.marks['extract'].update_xy(sp1d.spectral_axis.value,
+                                                sp1d.flux.value)
+        else:
+            self.marks['extract'].clear()
+
+        if self.interactive_extract and self.active_step == 'bg':
+            try:
+                spec = self.export_bg_spectrum()
+            except Exception:
+                self.marks['bg_spec'].clear()
+            else:
+                self.marks['bg_spec'].update_xy(spec.spectral_axis, spec.flux)
+        else:
+            self.marks['bg_spec'].clear()
+
+    @observe('is_active', 'trace_dataset_selected', 'trace_type_selected',
              'trace_trace_selected', 'trace_offset', 'trace_order',
              'trace_pixel', 'trace_peak_method_selected',
              'trace_do_binning', 'trace_bins', 'trace_window', 'active_step')
     def _interaction_in_trace_step(self, event={}):
-        if not self.is_active or not self._do_marks:
+        if not self._do_marks:
             return
-        if event.get('name', '') == 'active_step' and event.get('new') != 'trace':
+        if ((event.get('name', '') in ('active_step', 'is_active') and self.active_step != 'trace')
+                or not self.is_active):
+            self._update_plugin_marks(event)
             return
 
         try:
@@ -513,16 +543,21 @@ def _interaction_in_trace_step(self, event={}):
             self.marks['trace'].update_xy(range(len(trace.trace)),
                                           trace.trace)
             self.marks['trace'].line_style = 'solid'
+
+        self._update_interactive_extract(event)
+
         self.active_step = 'trace'
-        self._update_plugin_marks()
+        self._update_plugin_marks(event)
 
-    @observe('bg_dataset_selected', 'bg_type_selected',
+    @observe('is_active', 'bg_dataset_selected', 'bg_type_selected',
              'bg_trace_selected', 'bg_trace_pixel',
              'bg_separation', 'bg_width', 'bg_statistic_selected', 'active_step')
     def _interaction_in_bg_step(self, event={}):
-        if not self.is_active or not self._do_marks:
+        if not self._do_marks:
             return
-        if event.get('name', '') == 'active_step' and event.get('new') != 'bg':
+        if ((event.get('name', '') in ('active_step', 'is_active') and self.active_step != 'bg')
+                or not self.is_active):
+            self._update_plugin_marks(event)
             return
 
         try:
@@ -563,22 +598,19 @@ def _interaction_in_bg_step(self, event={}):
                 for mark in ['bg2_center', 'bg2_lower', 'bg2_upper']:
                     self.marks[mark].clear()
 
-        try:
-            spec = self.export_bg_spectrum()
-        except Exception:
-            self.marks['bg_spec'].clear()
-        else:
-            self.marks['bg_spec'].update_xy(spec.spectral_axis, spec.flux)
+        self._update_interactive_extract(event)
 
         self.active_step = 'bg'
-        self._update_plugin_marks()
+        self._update_plugin_marks(event)
 
-    @observe('ext_dataset_selected', 'ext_trace_selected',
+    @observe('is_active', 'ext_dataset_selected', 'ext_trace_selected',
              'ext_type_selected', 'ext_width', 'active_step')
     def _interaction_in_ext_step(self, event={}):
-        if not self.is_active or not self._do_marks:
+        if not self._do_marks:
             return
-        if event.get('name', '') == 'active_step' and event.get('new') != 'ext':
+        if ((event.get('name', '') in ('active_step', 'is_active') and self.active_step not in ('ext', ''))  # noqa
+                or not self.is_active):
+            self._update_plugin_marks(event)
             return
 
         try:
@@ -601,8 +633,10 @@ def _interaction_in_ext_step(self, event={}):
                 for mark in ['ext_lower', 'ext_upper']:
                     self.marks[mark].clear()
 
+        self._update_interactive_extract(event)
+
         self.active_step = 'ext'
-        self._update_plugin_marks()
+        self._update_plugin_marks(event)
 
         # TODO: remove this, the traitlet, and the row in spectral_extraction.vue
         # when specutils handles the warning/exception