From a805a7ba77cb2cada9c69d00cce9f3309679bcab Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 6 Jul 2023 14:35:00 -0400 Subject: [PATCH 1/7] Preserve plot options when replacing data from a plugin --- jdaviz/core/template_mixin.py | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 1b8fece455..732ee21242 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -1888,6 +1888,11 @@ def add_results_from_plugin(self, data_item, replace=None, label=None): Add ``data_item`` to the app's data_collection according to the default or user-provided label and adds to any requested viewers. """ + + # Note that we can only preserve one of percentile or vmin+vmax + preserve_attributes=("color", "alpha", "bias", "linewidth", "stretch", + "v_min", "v_max", "cmap") + if self.label_invalid_msg: raise ValueError(self.label_invalid_msg) @@ -1899,28 +1904,39 @@ def add_results_from_plugin(self, data_item, replace=None, label=None): # entry should be the same as the original entry (to avoid deleting reference data) add_to_viewer_refs = [] add_to_viewer_vis = [] + preserved_attributes = [] for viewer_select_item in self.add_to_viewer_items[1:]: # index 0 is for "None" viewer_ref = viewer_select_item['reference'] viewer_item = self.app._viewer_item_by_reference(viewer_ref) viewer = self.app.get_viewer(viewer_ref) - viewer_loaded_labels = [layer.layer.label for layer in viewer.layers] - if label in viewer_loaded_labels: - add_to_viewer_refs.append(viewer_ref) - add_to_viewer_vis.append(label in viewer_item['visible_layers']) + for layer in viewer.layers: + if layer.layer.label != label: + continue + else: + add_to_viewer_refs.append(viewer_ref) + add_to_viewer_vis.append(label in viewer_item['visible_layers']) + preserve_these = {} + for att in preserve_attributes: + if hasattr(layer.state, att): + preserve_these[att] = getattr(layer.state, att) + preserved_attributes.append(preserve_these) else: if self.add_to_viewer_selected == 'None': add_to_viewer_refs = [] add_to_viewer_vis = [] + preserved_attributes = [] else: add_to_viewer_refs = [self.add_to_viewer_selected] add_to_viewer_vis = [True] + preserved_attributes = [{}] if label in self.app.data_collection: for viewer_ref in add_to_viewer_refs: self.app.remove_data_from_viewer(viewer_ref, label) self.app.data_collection.remove(self.app.data_collection[label]) + if not hasattr(data_item, 'meta'): data_item.meta = {} data_item.meta['Plugin'] = self._plugin.__class__.__name__ @@ -1928,18 +1944,25 @@ def add_results_from_plugin(self, data_item, replace=None, label=None): data_item.meta['mosviz_row'] = self.app.state.settings['mosviz_row'] self.app.add_data(data_item, label) - for viewer_ref, visible in zip(add_to_viewer_refs, add_to_viewer_vis): + for viewer_ref, visible, preserved in zip(add_to_viewer_refs, add_to_viewer_vis, + preserved_attributes): # replace the contents in the selected viewer with the results from this plugin + this_viewer = self.app.get_viewer(viewer_ref) if replace is not None: this_replace = replace else: - this_viewer = self.app.get_viewer(viewer_ref) this_replace = isinstance(this_viewer, BqplotImageView) self.app.add_data_to_viewer(viewer_ref, label, visible=visible, clear_other_data=this_replace) + if preserved != {}: + layer_state = [layer.state for layer in this_viewer.layers if + layer.layer.label == label][0] + for att in preserved: + setattr(layer_state, att, preserved[att]) + # update overwrite warnings, etc self._on_label_changed() From 0f960ff18b44019c57fdeacc450e569f4bca8690 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 6 Jul 2023 15:14:32 -0400 Subject: [PATCH 2/7] Add changelog, fix codestyle --- CHANGES.rst | 3 +++ jdaviz/core/template_mixin.py | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 2593d792d8..84f9c3c701 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -109,6 +109,9 @@ Other Changes and Additions - Added direct launchers for each config (e.g. ``specviz``) [#1960] +- Replacing existing data from a plugin (e.g., refitting a model with the same label) + now preserves the plot options of the data as previously displayed. [#2288] + 3.5.1 (unreleased) ================== diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 732ee21242..af1149ba2c 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -1890,7 +1890,7 @@ def add_results_from_plugin(self, data_item, replace=None, label=None): """ # Note that we can only preserve one of percentile or vmin+vmax - preserve_attributes=("color", "alpha", "bias", "linewidth", "stretch", + preserve_attributes = ("color", "alpha", "bias", "linewidth", "stretch", "v_min", "v_max", "cmap") if self.label_invalid_msg: @@ -1936,7 +1936,6 @@ def add_results_from_plugin(self, data_item, replace=None, label=None): self.app.remove_data_from_viewer(viewer_ref, label) self.app.data_collection.remove(self.app.data_collection[label]) - if not hasattr(data_item, 'meta'): data_item.meta = {} data_item.meta['Plugin'] = self._plugin.__class__.__name__ From aad7a16daeebce10d7facacada59528fb73b2fbd Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 6 Jul 2023 16:24:46 -0400 Subject: [PATCH 3/7] Add test for refitting model --- .../model_fitting/tests/test_plugin.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py index 4c9d89e072..6471238323 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py @@ -154,6 +154,36 @@ def test_register_cube_model(cubeviz_helper, spectrum1d_cube): assert test_label in cubeviz_helper.app.data_collection +def test_refit_plot_options(specviz_helper, spectrum1d): + specviz_helper.load_data(spectrum1d) + modelfit_plugin = specviz_helper.plugins['Model Fitting'] + + model = MODELS['Const1D'] + modelfit_plugin.model_comp_selected = model + modelfit_plugin._obj.comp_label = f"C" + modelfit_plugin._obj.vue_add_model({}) + + with pytest.warns(AstropyUserWarning): + modelfit_plugin.calculate_fit() + + sv = specviz_helper.app.get_viewer('spectrum-viewer') + att_values = {} + atts = {"color": "red", "linewidth": 2, "alpha": 0.8} + layer_state = [layer.state for layer in sv.layers if layer.layer.label == "model"][0] + print(layer_state) + for att in atts: + setattr(layer_state, att, atts[att]) + + # Refit the model, which will replace the data by default. + with pytest.warns(AstropyUserWarning): + modelfit_plugin.calculate_fit() + + print(layer_state) + + for att in atts: + assert atts[att] == getattr(layer_state, att) + + def test_user_api(specviz_helper, spectrum1d): with warnings.catch_warnings(): warnings.simplefilter('ignore') From d40baf7e0dc7cde8fe1de48e567f91bcf12122be Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 6 Jul 2023 16:26:08 -0400 Subject: [PATCH 4/7] Codestyle --- .../default/plugins/model_fitting/tests/test_plugin.py | 5 ++--- jdaviz/core/template_mixin.py | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py index 6471238323..d55a6d9a11 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py @@ -160,14 +160,13 @@ def test_refit_plot_options(specviz_helper, spectrum1d): model = MODELS['Const1D'] modelfit_plugin.model_comp_selected = model - modelfit_plugin._obj.comp_label = f"C" + modelfit_plugin._obj.comp_label = "C" modelfit_plugin._obj.vue_add_model({}) with pytest.warns(AstropyUserWarning): modelfit_plugin.calculate_fit() sv = specviz_helper.app.get_viewer('spectrum-viewer') - att_values = {} atts = {"color": "red", "linewidth": 2, "alpha": 0.8} layer_state = [layer.state for layer in sv.layers if layer.layer.label == "model"][0] print(layer_state) @@ -181,7 +180,7 @@ def test_refit_plot_options(specviz_helper, spectrum1d): print(layer_state) for att in atts: - assert atts[att] == getattr(layer_state, att) + assert atts[att] == getattr(layer_state, att) def test_user_api(specviz_helper, spectrum1d): diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index af1149ba2c..0b3caa4b7f 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -1891,7 +1891,7 @@ def add_results_from_plugin(self, data_item, replace=None, label=None): # Note that we can only preserve one of percentile or vmin+vmax preserve_attributes = ("color", "alpha", "bias", "linewidth", "stretch", - "v_min", "v_max", "cmap") + "v_min", "v_max", "cmap") if self.label_invalid_msg: raise ValueError(self.label_invalid_msg) From 5d9f4490cc2f40701cd7c9356eab0a22a96de257 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Fri, 7 Jul 2023 09:16:08 -0400 Subject: [PATCH 5/7] Update test Update test so that it actually fails on main --- .../plugins/model_fitting/tests/test_plugin.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py index d55a6d9a11..2b287b1335 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py @@ -158,26 +158,26 @@ def test_refit_plot_options(specviz_helper, spectrum1d): specviz_helper.load_data(spectrum1d) modelfit_plugin = specviz_helper.plugins['Model Fitting'] - model = MODELS['Const1D'] - modelfit_plugin.model_comp_selected = model + modelfit_plugin.model_comp_selected = 'Const1D' modelfit_plugin._obj.comp_label = "C" modelfit_plugin._obj.vue_add_model({}) with pytest.warns(AstropyUserWarning): - modelfit_plugin.calculate_fit() + modelfit_plugin.calculate_fit(add_data=True) sv = specviz_helper.app.get_viewer('spectrum-viewer') atts = {"color": "red", "linewidth": 2, "alpha": 0.8} layer_state = [layer.state for layer in sv.layers if layer.layer.label == "model"][0] - print(layer_state) for att in atts: setattr(layer_state, att, atts[att]) - # Refit the model, which will replace the data by default. + # Refit using the same name, which will replace the data by default. + modelfit_plugin.create_model_component('Linear1D', 'L') + with pytest.warns(AstropyUserWarning): - modelfit_plugin.calculate_fit() + modelfit_plugin.calculate_fit(add_data=True) - print(layer_state) + layer_state = [layer.state for layer in sv.layers if layer.layer.label == "model"][0] for att in atts: assert atts[att] == getattr(layer_state, att) From 2b5eb7bf1b6354fc28e79f0cf891586ee01164b7 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Tue, 11 Jul 2023 16:42:53 -0400 Subject: [PATCH 6/7] Switch to attribute ignore list --- jdaviz/core/template_mixin.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 0b3caa4b7f..3a796c9ef9 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -1890,8 +1890,7 @@ def add_results_from_plugin(self, data_item, replace=None, label=None): """ # Note that we can only preserve one of percentile or vmin+vmax - preserve_attributes = ("color", "alpha", "bias", "linewidth", "stretch", - "v_min", "v_max", "cmap") + ignore_attributes = ("layer", "attribute") if self.label_invalid_msg: raise ValueError(self.label_invalid_msg) @@ -1917,8 +1916,8 @@ def add_results_from_plugin(self, data_item, replace=None, label=None): add_to_viewer_refs.append(viewer_ref) add_to_viewer_vis.append(label in viewer_item['visible_layers']) preserve_these = {} - for att in preserve_attributes: - if hasattr(layer.state, att): + for att in layer.state.as_dict(): + if att not in ignore_attributes: preserve_these[att] = getattr(layer.state, att) preserved_attributes.append(preserve_these) else: From 658ee7bf9150fa048c09a876dfdcb9f7f5fd5f56 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 13 Jul 2023 10:24:23 -0400 Subject: [PATCH 7/7] Ignore other attributes that we can't set --- jdaviz/core/template_mixin.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 3a796c9ef9..d1d04eee2d 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -1890,7 +1890,7 @@ def add_results_from_plugin(self, data_item, replace=None, label=None): """ # Note that we can only preserve one of percentile or vmin+vmax - ignore_attributes = ("layer", "attribute") + ignore_attributes = ("layer", "attribute", "percentile") if self.label_invalid_msg: raise ValueError(self.label_invalid_msg) @@ -1917,7 +1917,8 @@ def add_results_from_plugin(self, data_item, replace=None, label=None): add_to_viewer_vis.append(label in viewer_item['visible_layers']) preserve_these = {} for att in layer.state.as_dict(): - if att not in ignore_attributes: + # Can't set cmap_att, size_att, etc + if att not in ignore_attributes and "_att" not in att: preserve_these[att] = getattr(layer.state, att) preserved_attributes.append(preserve_these) else: