From 960b8a6b20d3cfcb2edc5efe7a8121efe06a3fd5 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Mon, 1 Jul 2024 15:18:28 -0400 Subject: [PATCH 1/4] support exporting a plot from an unopened/inactive plugin --- jdaviz/configs/default/plugins/export/export.py | 14 +++++++++----- .../default/plugins/plot_options/plot_options.py | 13 ++++++++++--- jdaviz/core/template_mixin.py | 9 ++++++++- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/jdaviz/configs/default/plugins/export/export.py b/jdaviz/configs/default/plugins/export/export.py index f6db90b635..a03899f383 100644 --- a/jdaviz/configs/default/plugins/export/export.py +++ b/jdaviz/configs/default/plugins/export/export.py @@ -73,6 +73,7 @@ class Export(PluginTemplateMixin, ViewerSelectMixin, SubsetSelectMixin, dataset_format_items = List().tag(sync=True) dataset_format_selected = Unicode().tag(sync=True) + plugin_plot_selected_widget = Unicode().tag(sync=True) # copy of widget of the selected plugin_plot in case the parent plugin is not opened plugin_plot_format_items = List().tag(sync=True) plugin_plot_format_selected = Unicode().tag(sync=True) @@ -462,11 +463,14 @@ def export(self, filename=None, show_dialog=None, overwrite=False, else: filename = None - with plot._plugin.as_active(): - # NOTE: could still take some time for the plot itself to update, - # for now we'll hardcode a short amount of time for the plot to render any updates - time.sleep(0.2) - self.save_figure(plot, filename, filetype, show_dialog=show_dialog) + if not plot._plugin.is_active: + # force an update to the plot. This requires the plot to have set update_callback when instantiated + plot._update() + + # create a copy of the widget shown off screen to enable rendering in case one was never created in the parent plugin + self.plugin_plot_selected_widget = f'IPY_MODEL_{plot.model_id}' + + self.save_figure(plot, filename, filetype, show_dialog=show_dialog) elif len(self.plugin_table.selected): filetype = self.plugin_table_format.selected diff --git a/jdaviz/configs/default/plugins/plot_options/plot_options.py b/jdaviz/configs/default/plugins/plot_options/plot_options.py index aaf7a3ddb0..bdb8f51015 100644 --- a/jdaviz/configs/default/plugins/plot_options/plot_options.py +++ b/jdaviz/configs/default/plugins/plot_options/plot_options.py @@ -567,7 +567,8 @@ def state_attr_for_line_visible(state): 'stretch_params_value', 'stretch_params_sync', state_filter=is_image) - self.stretch_histogram = Plot(self, name='stretch_hist', viewer_type='histogram') + self.stretch_histogram = Plot(self, name='stretch_hist', viewer_type='histogram', + update_callback=self._update_stretch_histogram) # Add the stretch bounds tool to the default Plot viewer. self.stretch_histogram.tools_nested.append(["jdaviz:stretch_bounds"]) self.stretch_histogram._initialize_toolbar(["jdaviz:stretch_bounds"]) @@ -886,8 +887,7 @@ def _update_stretch_hist_sync(self, msg={}): @observe('is_active', 'layer_selected', 'viewer_selected', 'stretch_hist_zoom_limits') @skip_if_no_updates_since_last_active() - @with_spinner('stretch_hist_spinner') - def _update_stretch_histogram(self, msg={}): + def _request_update_stretch_histogram(self, msg={}): if not hasattr(self, 'viewer'): # pragma: no cover # plugin hasn't been fully initialized yet return @@ -903,6 +903,13 @@ def _update_stretch_histogram(self, msg={}): # its type msg = {} + # NOTE: this method is separate from _update_stretch_histogram so that _update_stretch_histogram + # can be called manually (or from the update_callback on the Plot object itself) without going through + # the skip_if_no_updates_since_last_active check + self._update_stretch_histogram(msg) + + @with_spinner('stretch_hist_spinner') + def _update_stretch_histogram(self, msg={}): if not self.stretch_function_sync.get('in_subscribed_states'): # pragma: no cover # no (image) viewer with stretch function options return diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 12dfe2d49d..ddc61297ec 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -4685,12 +4685,14 @@ class Plot(PluginSubcomponent): figure = Any().tag(sync=True, **widget_serialization) toolbar = Any().tag(sync=True, **widget_serialization) - def __init__(self, plugin, name='plot', viewer_type='scatter', app=None, *args, **kwargs): + def __init__(self, plugin, name='plot', viewer_type='scatter', update_callback=None, + app=None, *args, **kwargs): super().__init__(plugin, 'Plot', *args, **kwargs) if app is None: app = jglue() self._app = app + self._update_callback = update_callback self._plugin = plugin self._plot_name = name self.viewer = app.new_data_viewer(viewer_type, show=False) @@ -4748,6 +4750,11 @@ def _remove_data(self, label): self._plugin.session.hub.broadcast(PluginPlotModifiedMessage(sender=self)) + def _update(self): + # call the update callback, if it exists, on the parent plugin. This is useful for updating the plot when a plugin is inactive + if self._update_callback is not None: + self._update_callback() + def _update_data(self, label, reset_lims=False, **kwargs): self._check_valid_components(**kwargs) if label not in self.app.data_collection: From 1c90461dcb89a584d756c9ca06d66a15518a30e6 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Tue, 2 Jul 2024 08:24:23 -0400 Subject: [PATCH 2/4] changelog entry --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 122fd1461c..b41a3ca757 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -190,6 +190,8 @@ Bug Fixes - Display default filepath in Export plugin, re-enable API exporting, enable relative and absolute path exports from the UI. [#2896] +- Fixes exporting the stretch histogram from Plot Options before the Plot Options plugin is ever opened. [#2934] + Cubeviz ^^^^^^^ From f4400642eee62e0f1d42649c2373658d5f9597fd Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Tue, 2 Jul 2024 08:26:51 -0400 Subject: [PATCH 3/4] update comments --- jdaviz/configs/default/plugins/export/export.py | 10 +++++++--- .../default/plugins/plot_options/plot_options.py | 5 +++-- jdaviz/core/template_mixin.py | 3 ++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/jdaviz/configs/default/plugins/export/export.py b/jdaviz/configs/default/plugins/export/export.py index a03899f383..1538c6794a 100644 --- a/jdaviz/configs/default/plugins/export/export.py +++ b/jdaviz/configs/default/plugins/export/export.py @@ -73,7 +73,9 @@ class Export(PluginTemplateMixin, ViewerSelectMixin, SubsetSelectMixin, dataset_format_items = List().tag(sync=True) dataset_format_selected = Unicode().tag(sync=True) - plugin_plot_selected_widget = Unicode().tag(sync=True) # copy of widget of the selected plugin_plot in case the parent plugin is not opened + # copy of widget of the selected plugin_plot in case the parent plugin is not opened + plugin_plot_selected_widget = Unicode().tag(sync=True) + plugin_plot_format_items = List().tag(sync=True) plugin_plot_format_selected = Unicode().tag(sync=True) @@ -464,10 +466,12 @@ def export(self, filename=None, show_dialog=None, overwrite=False, filename = None if not plot._plugin.is_active: - # force an update to the plot. This requires the plot to have set update_callback when instantiated + # force an update to the plot. This requires the plot to have set + # update_callback when instantiated plot._update() - # create a copy of the widget shown off screen to enable rendering in case one was never created in the parent plugin + # create a copy of the widget shown off screen to enable rendering + # in case one was never created in the parent plugin self.plugin_plot_selected_widget = f'IPY_MODEL_{plot.model_id}' self.save_figure(plot, filename, filetype, show_dialog=show_dialog) diff --git a/jdaviz/configs/default/plugins/plot_options/plot_options.py b/jdaviz/configs/default/plugins/plot_options/plot_options.py index bdb8f51015..d77aa30451 100644 --- a/jdaviz/configs/default/plugins/plot_options/plot_options.py +++ b/jdaviz/configs/default/plugins/plot_options/plot_options.py @@ -903,8 +903,9 @@ def _request_update_stretch_histogram(self, msg={}): # its type msg = {} - # NOTE: this method is separate from _update_stretch_histogram so that _update_stretch_histogram - # can be called manually (or from the update_callback on the Plot object itself) without going through + # NOTE: this method is separate from _update_stretch_histogram so that + # _update_stretch_histogram can be called manually (or from the + # update_callback on the Plot object itself) without going through # the skip_if_no_updates_since_last_active check self._update_stretch_histogram(msg) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index ddc61297ec..443e59bc3e 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -4751,7 +4751,8 @@ def _remove_data(self, label): self._plugin.session.hub.broadcast(PluginPlotModifiedMessage(sender=self)) def _update(self): - # call the update callback, if it exists, on the parent plugin. This is useful for updating the plot when a plugin is inactive + # call the update callback, if it exists, on the parent plugin. + # This is useful for updating the plot when a plugin is inactive. if self._update_callback is not None: self._update_callback() From b89689a9e3b3d1087634e896b56189a4c7e51a44 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Tue, 2 Jul 2024 12:48:57 -0400 Subject: [PATCH 4/4] prevent clearing plot selection * when the plot was being updated, a plot update message was being sent when clearing the old data, briefly resetting the choices to an empty list, which cleared the selection. The broadcast now is not duplicated and so the choices remains intact --- jdaviz/core/template_mixin.py | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 443e59bc3e..07dae242d8 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -949,6 +949,7 @@ def _apply_default_selection(self, skip_if_current_valid=True): self.selected = self._default_text if self._default_text else default_empty else: self.selected = default_empty + self._clear_cache(*self._cached_properties) def _is_valid_item(self, item, filter_callables={}): for valid_filter in self.filters: @@ -2632,9 +2633,7 @@ def _on_tables_changed(self, *args): manual_items = [{'label': label} for label in self.manual_options] self.items = manual_items + [{'label': k} for k, v in self.plugin.app._plugin_tables.items() if self._is_valid_item(v._obj)] - self._apply_default_selection() - # future improvement: only clear cache if the selected data entry was changed? - self._clear_cache(*self._cached_properties) + self._apply_default_selection(skip_if_current_valid=True) @cached_property def selected_obj(self): @@ -2747,8 +2746,7 @@ def _on_plots_changed(self, *args): manual_items = [{'label': label} for label in self.manual_options] self.items = manual_items + [{'label': k} for k, v in self.plugin.app._plugin_plots.items() if self._is_valid_item(v._obj)] - self._apply_default_selection() - # future improvement: only clear cache if the selected data entry was changed? + self._apply_default_selection(skip_if_current_valid=True) self._clear_cache(*self._cached_properties) @cached_property @@ -2756,7 +2754,6 @@ def selected_obj(self): return self.plugin.app._jdaviz_helper.plugin_plots.get(self.selected) def _is_valid_item(self, plot): - def not_empty_plot(plot): # checks plot.figure.marks to determine if figure is of an empty plot # not sure if this is a foolproof way to do this? @@ -4717,7 +4714,6 @@ def __init__(self, plugin, name='plot', viewer_type='scatter', update_callback=N self._initialize_toolbar() plugin.session.hub.broadcast(PluginPlotAddedMessage(sender=self)) - plugin.session.hub.broadcast(PluginPlotModifiedMessage(sender=self)) def _initialize_toolbar(self, default_tool_priority=[]): self.toolbar = NestedJupyterToolbar(self.viewer, self.tools_nested, default_tool_priority) @@ -4743,12 +4739,13 @@ def _check_valid_components(self, **kwargs): # https://github.com/astrofrog/fast-histogram/issues/60 raise ValueError("histogram requires data entries with length > 1") - def _remove_data(self, label): + def _remove_data(self, label, broadcast=True): dc_entry = self.app.data_collection[label] self.viewer.remove_data(dc_entry) self.app.data_collection.remove(dc_entry) - self._plugin.session.hub.broadcast(PluginPlotModifiedMessage(sender=self)) + if broadcast: + self._plugin.session.hub.broadcast(PluginPlotModifiedMessage(sender=self)) def _update(self): # call the update callback, if it exists, on the parent plugin. @@ -4782,8 +4779,8 @@ def _update_data(self, label, reset_lims=False, **kwargs): style_state = self.layers[label].state.as_dict() else: style_state = {} - self._remove_data(label) - self._add_data(label, **kwargs) + self._remove_data(label, broadcast=False) + self._add_data(label, broadcast=False, **kwargs) self.update_style(label, **style_state) if reset_lims: self.viewer.state.reset_limits() @@ -4819,7 +4816,7 @@ def update_style(self, label, **kwargs): self._plugin.session.hub.broadcast(PluginPlotModifiedMessage(sender=self)) - def _add_data(self, label, **kwargs): + def _add_data(self, label, broadcast=True, **kwargs): self._check_valid_components(**kwargs) data = Data(label=label, **kwargs) dc = self.app.data_collection @@ -4834,7 +4831,8 @@ def _add_data(self, label, **kwargs): dc.add_link(links) self.viewer.add_data(dc_entry) - self._plugin.session.hub.broadcast(PluginPlotModifiedMessage(sender=self)) + if broadcast: + self._plugin.session.hub.broadcast(PluginPlotModifiedMessage(sender=self)) def _refresh_marks(self): # ensure all marks are drawn