From 644c6fa1987251068304f03d742ace0bb5ce573b Mon Sep 17 00:00:00 2001 From: Clare Shanahan <cshanahan@stsci.edu> Date: Tue, 12 Dec 2023 16:07:43 -0500 Subject: [PATCH 01/37] adding check for subset name --- jdaviz/app.py | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/jdaviz/app.py b/jdaviz/app.py index a8b2de2cd4..8c7049203e 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -1514,6 +1514,47 @@ def add_data(self, data, data_label=None, notify_done=True, parent=None): f"Data '{data_label}' successfully added.", sender=self, color="success") self.hub.broadcast(snackbar_message) + def check_valid_subset_label(self, subset_name): + """Check that `subset_name` is a valid choice for a subset name. This + check is run when renaming subsets. + + A valid subset name must not be the name of another subset in the data + collection (case insensitive? there cant be a subset 1 and a Subset 1.) + The name may match a subset in the data collection if it the current + active selection (i.e the renaming is not really a renaming, it is + just keeping the old name, which is valid). + + Unlike dataset names, the attempted renaming of a subset to an existing + subset label will not append a number (e.g Subset 1 repeated because + Subset 1(1)). If the name exists, a warning will be raised and the + original subset name will be returned. + + """ + + # get all existing subset labels + dc = self.data_collection + subsets = dc.subset_groups + existing_labels = [subset.label.lower() for subset in subsets] + + # get active selection, if there is one + if self.session.edit_subset_mode.edit_subset == []: + subset_selected = None + else: + subset_selected = self.session.edit_subset_mode.edit_subset[0].label + + # remove the current selection label from the set of labels, because its ok + # if the new subset shares the name of the current selection (renaming to current name) + if subset_selected.lower() in existing_labels: + existing_labels.remove(subset_selected.lower()) + + # now check `subset_name` against list of non-active current subset labels + # and warn and return if it is + if subset_name.lower() in existing_labels: + warnings.warn(f"Can not rename subset to name of another existing subset ({subset_name}).") + return subset_selected + + return subset_name + def return_data_label(self, loaded_object, ext=None, alt_name=None, check_unique=True): """ Returns a unique data label that can be safely used to load data into data collection. From 33c29a46b52a2f1ef5b1919da64cca6fa30333a2 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Tue, 10 Dec 2024 11:49:41 -0500 Subject: [PATCH 02/37] Create list of reserved labels at app level to check against --- jdaviz/app.py | 28 +++++++++++++++++----------- jdaviz/core/template_mixin.py | 5 +++++ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 8c7049203e..574dfc90cc 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -319,6 +319,9 @@ def __init__(self, configuration=None, *args, **kwargs): self.hub.subscribe(self, PluginPlotAddedMessage, handler=self._on_plugin_plot_added) + # Convenient reference of all existing subset names + self._reserved_labels = set([None,]) + # Parse the yaml configuration file used to compose the front-end UI self.load_configuration(configuration) @@ -371,10 +374,14 @@ def __init__(self, configuration=None, *args, **kwargs): # Internal cache so we don't have to keep calling get_object for the same Data. # Key should be (data_label, statistic) and value the translated object. self._get_object_cache = {} + self.hub.subscribe(self, SubsetUpdateMessage, handler=self._on_subset_update_message) + # These both call _on_layers_changed self.hub.subscribe(self, SubsetDeleteMessage, handler=self._on_subset_delete_message) + self.hub.subscribe(self, SubsetCreateMessage, + handler=self._on_subset_create_message) # Store for associations between Data entries: self._data_associations = self._init_data_associations() @@ -384,9 +391,6 @@ def __init__(self, configuration=None, *args, **kwargs): handler=self._on_add_data_message) self.hub.subscribe(self, RemoveDataMessage, handler=self._on_layers_changed) - self.hub.subscribe(self, SubsetCreateMessage, - handler=self._on_layers_changed) - # SubsetDeleteMessage will also call _on_layers_changed via _on_subset_delete_message # Emit messages when icons are updated self.state.add_callback('viewer_icons', @@ -457,6 +461,7 @@ def _on_add_data_message(self, msg): self._update_live_plugin_results(trigger_data_lbl=msg.data.label) def _on_subset_update_message(self, msg): + print(msg) # NOTE: print statements in here will require the viewer output_widget self._clear_object_cache(msg.subset.label) if msg.attribute == 'subset_state': @@ -464,6 +469,12 @@ def _on_subset_update_message(self, msg): def _on_subset_delete_message(self, msg): self._remove_live_plugin_results(trigger_subset=msg.subset) + self._reserved_labels.remove(msg.subset.label.lower()) + self._on_layers_changed(msg) + + def _on_subset_create_message(self, msg): + print(msg) + self._reserved_labels.add(msg.subset.label.lower()) self._on_layers_changed(msg) def _on_plugin_plot_added(self, msg): @@ -1531,11 +1542,6 @@ def check_valid_subset_label(self, subset_name): """ - # get all existing subset labels - dc = self.data_collection - subsets = dc.subset_groups - existing_labels = [subset.label.lower() for subset in subsets] - # get active selection, if there is one if self.session.edit_subset_mode.edit_subset == []: subset_selected = None @@ -1544,12 +1550,12 @@ def check_valid_subset_label(self, subset_name): # remove the current selection label from the set of labels, because its ok # if the new subset shares the name of the current selection (renaming to current name) - if subset_selected.lower() in existing_labels: - existing_labels.remove(subset_selected.lower()) + if subset_selected.lower() in self._reserved_labels: + self._reserved_labels.remove(subset_selected.lower()) # now check `subset_name` against list of non-active current subset labels # and warn and return if it is - if subset_name.lower() in existing_labels: + if subset_name.lower() in self._reserved_labels: warnings.warn(f"Can not rename subset to name of another existing subset ({subset_name}).") return subset_selected diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 478d89f32e..f07e7f878e 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -791,6 +791,11 @@ def __init__(self, *args, **kwargs): manual_options = [default_text] + manual_options self._manual_options = manual_options + # Reserve the default and manual options strings so people can't use them as Subset labels + self._plugin.app._reserved_labels.add(str(default_text).lower()) + self._plugin.app._reserved_labels.update([x["label"].lower() if isinstance(x, dict) else + x.lower() for x in manual_options]) + self.items = [self._to_item(opt) for opt in manual_options] # set default values for traitlets if default_text is not None: From d276f7c8103ca1c8cb68f1fc039706a055027713 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Wed, 11 Dec 2024 11:12:44 -0500 Subject: [PATCH 03/37] Force labels to strings now so don't need None --- jdaviz/app.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 574dfc90cc..0f998b2000 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -320,7 +320,7 @@ def __init__(self, configuration=None, *args, **kwargs): handler=self._on_plugin_plot_added) # Convenient reference of all existing subset names - self._reserved_labels = set([None,]) + self._reserved_labels = set([]) # Parse the yaml configuration file used to compose the front-end UI self.load_configuration(configuration) @@ -2035,6 +2035,10 @@ def _get_viewer_item(self, ref_or_id): viewer_item = self._viewer_item_by_id(ref_or_id) return viewer_item + def _rename_subset(self, old_label, new_label): + # Change the label of a subset, making sure it propagates to as many places as it can + pass + def _reparent_subsets(self, old_parent, new_parent=None): ''' Re-parent subsets that belong to the specified data From 8a760f9b886b77a32d4b725f140dca947b3cdf60 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Thu, 12 Dec 2024 12:56:08 -0500 Subject: [PATCH 04/37] Move validation function, minimize layer reprocessing, start adding to subset_tools --- jdaviz/app.py | 88 +++++++++++-------- .../plugins/subset_tools/subset_tools.py | 9 +- 2 files changed, 55 insertions(+), 42 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 0f998b2000..356372131b 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -461,7 +461,6 @@ def _on_add_data_message(self, msg): self._update_live_plugin_results(trigger_data_lbl=msg.data.label) def _on_subset_update_message(self, msg): - print(msg) # NOTE: print statements in here will require the viewer output_widget self._clear_object_cache(msg.subset.label) if msg.attribute == 'subset_state': @@ -473,7 +472,6 @@ def _on_subset_delete_message(self, msg): self._on_layers_changed(msg) def _on_subset_create_message(self, msg): - print(msg) self._reserved_labels.add(msg.subset.label.lower()) self._on_layers_changed(msg) @@ -595,6 +593,9 @@ def _on_layers_changed(self, msg): children_layers = self._get_assoc_data_children(layer_name) elif hasattr(msg, 'subset'): + # We don't need to reprocess the subset for every data collection entry + if msg.subset.data != self.data_collection[0]: + return layer_name = msg.subset.label is_wcs_only = False is_not_child = True @@ -1525,42 +1526,6 @@ def add_data(self, data, data_label=None, notify_done=True, parent=None): f"Data '{data_label}' successfully added.", sender=self, color="success") self.hub.broadcast(snackbar_message) - def check_valid_subset_label(self, subset_name): - """Check that `subset_name` is a valid choice for a subset name. This - check is run when renaming subsets. - - A valid subset name must not be the name of another subset in the data - collection (case insensitive? there cant be a subset 1 and a Subset 1.) - The name may match a subset in the data collection if it the current - active selection (i.e the renaming is not really a renaming, it is - just keeping the old name, which is valid). - - Unlike dataset names, the attempted renaming of a subset to an existing - subset label will not append a number (e.g Subset 1 repeated because - Subset 1(1)). If the name exists, a warning will be raised and the - original subset name will be returned. - - """ - - # get active selection, if there is one - if self.session.edit_subset_mode.edit_subset == []: - subset_selected = None - else: - subset_selected = self.session.edit_subset_mode.edit_subset[0].label - - # remove the current selection label from the set of labels, because its ok - # if the new subset shares the name of the current selection (renaming to current name) - if subset_selected.lower() in self._reserved_labels: - self._reserved_labels.remove(subset_selected.lower()) - - # now check `subset_name` against list of non-active current subset labels - # and warn and return if it is - if subset_name.lower() in self._reserved_labels: - warnings.warn(f"Can not rename subset to name of another existing subset ({subset_name}).") - return subset_selected - - return subset_name - def return_data_label(self, loaded_object, ext=None, alt_name=None, check_unique=True): """ Returns a unique data label that can be safely used to load data into data collection. @@ -2035,9 +2000,53 @@ def _get_viewer_item(self, ref_or_id): viewer_item = self._viewer_item_by_id(ref_or_id) return viewer_item + def _check_valid_subset_label(self, subset_name, warn_if_duplicate=True): + """Check that `subset_name` is a valid choice for a subset name. This + check is run when renaming subsets. + + A valid subset name must not be the name of another subset in the data + collection (case insensitive? there cant be a subset 1 and a Subset 1.) + The name may match a subset in the data collection if it the current + active selection (i.e the renaming is not really a renaming, it is + just keeping the old name, which is valid). + + Unlike dataset names, the attempted renaming of a subset to an existing + subset label will not append a number (e.g Subset 1 repeated because + Subset 1(1)). If the name exists, a warning will be raised and the + original subset name will be returned. + + """ + + # get active selection, if there is one + if self.session.edit_subset_mode.edit_subset == []: + subset_selected = None + else: + subset_selected = self.session.edit_subset_mode.edit_subset[0].label + + # remove the current selection label from the set of labels, because its ok + # if the new subset shares the name of the current selection (renaming to current name) + if subset_selected.lower() in self._reserved_labels: + self._reserved_labels.remove(subset_selected.lower()) + + # now check `subset_name` against list of non-active current subset labels + # and warn and return if it is + if subset_name.lower() in self._reserved_labels: + if warn_if_duplicate: + warnings.warn(f"Can not rename subset to name of another existing subset ({subset_name}).") + return False + + return True + def _rename_subset(self, old_label, new_label): # Change the label of a subset, making sure it propagates to as many places as it can - pass + # I don't think there's an easier way to get subset_group by label, it's just a tuple + for s in self.data_collection.subset_groups: + if s.label == old_label: + sg = s + break + print(sg) + if self._check_valid_subset_label(new_label): + sg.label = new_label def _reparent_subsets(self, old_parent, new_parent=None): ''' @@ -2431,6 +2440,7 @@ def _on_data_added(self, msg): self._link_new_data() data_item = self._create_data_item(msg.data) self.state.data_items.append(data_item) + self._reserved_labels.add(msg.data.label) def _clear_object_cache(self, data_label=None): if data_label is None: diff --git a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py index 4e9dc5290a..21dba8fad4 100644 --- a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py @@ -300,8 +300,11 @@ def _sync_selected_from_state(self, *args): self.show_region_info = True self._update_combination_mode() - def _on_subset_update(self, *args): - self._sync_selected_from_state(*args) + def _on_subset_update(self, msg): + if msg.attribute == "label": + print("updating label") + print(msg.subset.label) + self._sync_selected_from_state() if 'Create New' in self.subset_selected: return subsets_avail = [sg.label for sg in self.app.data_collection.subset_groups] @@ -309,7 +312,7 @@ def _on_subset_update(self, *args): # subset selection should re-default after processing the deleted subset, # for now we can safely ignore return - self._get_subset_definition(*args) + self._get_subset_definition() subset_to_update = self.session.edit_subset_mode.edit_subset[0] self.subset._update_subset(subset_to_update, attribute="type") From 354d9f819443d1ca8fb4c32b80683ecac4190e7d Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Thu, 12 Dec 2024 13:45:52 -0500 Subject: [PATCH 05/37] Warn if label is not alphanumberic, adding code from #1291 Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> --- jdaviz/app.py | 27 ++++++++++++------- .../plugins/subset_tools/subset_tools.py | 27 +++++++++++++++++-- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 356372131b..08ca7db137 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2000,7 +2000,7 @@ def _get_viewer_item(self, ref_or_id): viewer_item = self._viewer_item_by_id(ref_or_id) return viewer_item - def _check_valid_subset_label(self, subset_name, warn_if_duplicate=True): + def _check_valid_subset_label(self, subset_name, warn_if_invalid=True): """Check that `subset_name` is a valid choice for a subset name. This check is run when renaming subsets. @@ -2031,22 +2031,29 @@ def _check_valid_subset_label(self, subset_name, warn_if_duplicate=True): # now check `subset_name` against list of non-active current subset labels # and warn and return if it is if subset_name.lower() in self._reserved_labels: - if warn_if_duplicate: - warnings.warn(f"Can not rename subset to name of another existing subset ({subset_name}).") + if warn_if_invalid: + warnings.warn(f"Can not rename subset to name of another existing subset" + " or data item: ({subset_name}).") + return False + + elif not subset_name.replace(" ", "").isalnum(): + if warn_if_invalid: + warnings.warn("Subset labels must be purely alphanumeric") return False return True - def _rename_subset(self, old_label, new_label): + def _rename_subset(self, old_label, new_label, subset_group=None, check_if_valid): # Change the label of a subset, making sure it propagates to as many places as it can # I don't think there's an easier way to get subset_group by label, it's just a tuple - for s in self.data_collection.subset_groups: - if s.label == old_label: - sg = s - break - print(sg) + if subset_group is None: + for s in self.data_collection.subset_groups: + if s.label == old_label: + subset_group = s + break + print(subset_group) if self._check_valid_subset_label(new_label): - sg.label = new_label + subset_group.label = new_label def _reparent_subsets(self, old_parent, new_parent=None): ''' diff --git a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py index 21dba8fad4..b2e9990ee2 100644 --- a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py @@ -4,6 +4,7 @@ from astropy.time import Time import astropy.units as u +from functools import cached_property from glue.core.message import EditSubsetMessage, SubsetUpdateMessage from glue.core.edit_subset_mode import (AndMode, AndNotMode, OrMode, ReplaceMode, XorMode, NewMode) @@ -166,8 +167,11 @@ def __init__(self, *args, **kwargs): @property def user_api(self): - expose = ['subset', 'combination_mode', 'recenter_dataset', 'recenter', - 'get_center', 'set_center', 'import_region', 'get_regions'] + expose = ['subset', 'combination_mode', + 'recenter_dataset', 'recenter', + 'get_center', 'set_center', + 'import_region', 'get_regions', + 'rename_subset'] return PluginUserApi(self, expose) def get_regions(self, region_type=None, list_of_subset_labels=None, @@ -301,9 +305,11 @@ def _sync_selected_from_state(self, *args): self._update_combination_mode() def _on_subset_update(self, msg): + ''' if msg.attribute == "label": print("updating label") print(msg.subset.label) + ''' self._sync_selected_from_state() if 'Create New' in self.subset_selected: return @@ -842,6 +848,23 @@ def _set_value_in_subset_definition(self, index, name, desired_key, new_value): self.subset_definitions[index][i]['value'] = new_value break + @cached_property + def selected_subset_group(self): + for subset_group in self.app.data_collection.subset_groups: + if subset_group.label == self.selected: + return subset_group + + def rename_selected(self, new_name): + if new_name in self.labels: + raise ValueError(f"{new_name} is already a named subset") + if new_name in ['Entire Spectrum', 'Surrounding']: + # these are names used in various subset dropdowns for other meanings + raise ValueError(f"{new_name} is a reserved name") + + subset_group = self.selected_subset_group + if subset_group is None: + raise TypeError("current selection is not a subset") + def import_region(self, region, combination_mode=None, max_num_regions=None, refdata_label=None, return_bad_regions=False, **kwargs): """ From 68bcf15675e9928389ba01e00ccb9ff5a0ffd743 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Fri, 13 Dec 2024 11:23:20 -0500 Subject: [PATCH 06/37] Add SubsetRenameMessage --- jdaviz/core/events.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/jdaviz/core/events.py b/jdaviz/core/events.py index f4b3e8baf4..c1d0a8c810 100644 --- a/jdaviz/core/events.py +++ b/jdaviz/core/events.py @@ -2,7 +2,7 @@ from glue.core.message import Message __all__ = ['NewViewerMessage', 'ViewerAddedMessage', 'ViewerRemovedMessage', 'LoadDataMessage', - 'AddDataMessage', 'SnackbarMessage', 'RemoveDataMessage', + 'AddDataMessage', 'SnackbarMessage', 'RemoveDataMessage', 'SubsetRenameMessage', 'AddLineListMessage', 'RowLockMessage', 'SliceSelectSliceMessage', 'SliceValueUpdatedMessage', 'SliceToolStateMessage', @@ -161,6 +161,27 @@ def old(self): return self._old +class SubsetRenameMessage(Message): + def __init__(self, subset_group, old_label, new_label, *args, **kwargs): + super().__init__(*args, **kwargs) + + self._subset_group = subset_group + self._old_label = old_label + self._new_label = new_label + + @property + def subset_group(self): + return self._subset_group + + @property + def old_label(self): + return self._old_label + + @property + def new_label(self): + return self._new_label + + class SnackbarMessage(Message): def __init__(self, text, color=None, timeout=5000, loading=False, *args, **kwargs): From fa75705b467244e45c9fe0f1811a9719dae455a3 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Fri, 13 Dec 2024 13:55:17 -0500 Subject: [PATCH 07/37] Update all SubsetSelects in plugins when subset is renamed. --- jdaviz/app.py | 11 +++++--- .../plugins/subset_tools/subset_tools.py | 18 +++++++------ jdaviz/core/template_mixin.py | 26 ++++++++++++++++++- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 08ca7db137..13d5c219d8 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -41,7 +41,7 @@ from jdaviz import style_registry from jdaviz.core.config import read_configuration, get_configuration from jdaviz.core.events import (LoadDataMessage, NewViewerMessage, AddDataMessage, - SnackbarMessage, RemoveDataMessage, + SnackbarMessage, RemoveDataMessage, SubsetRenameMessage, AddDataToViewerMessage, RemoveDataFromViewerMessage, ViewerAddedMessage, ViewerRemovedMessage, ViewerRenamedMessage, ChangeRefDataMessage, @@ -2043,7 +2043,7 @@ def _check_valid_subset_label(self, subset_name, warn_if_invalid=True): return True - def _rename_subset(self, old_label, new_label, subset_group=None, check_if_valid): + def _rename_subset(self, old_label, new_label, subset_group=None, check_valid=True): # Change the label of a subset, making sure it propagates to as many places as it can # I don't think there's an easier way to get subset_group by label, it's just a tuple if subset_group is None: @@ -2052,9 +2052,14 @@ def _rename_subset(self, old_label, new_label, subset_group=None, check_if_valid subset_group = s break print(subset_group) - if self._check_valid_subset_label(new_label): + if check_valid: + if self._check_valid_subset_label(new_label): + subset_group.label = new_label + else: subset_group.label = new_label + self.hub.broadcast(SubsetRenameMessage(subset_group, old_label, new_label, sender=self)) + def _reparent_subsets(self, old_parent, new_parent=None): ''' Re-parent subsets that belong to the specified data diff --git a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py index b2e9990ee2..01b759d131 100644 --- a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py @@ -171,7 +171,7 @@ def user_api(self): 'recenter_dataset', 'recenter', 'get_center', 'set_center', 'import_region', 'get_regions', - 'rename_subset'] + 'rename_selected', 'rename_subset'] return PluginUserApi(self, expose) def get_regions(self, region_type=None, list_of_subset_labels=None, @@ -851,20 +851,22 @@ def _set_value_in_subset_definition(self, index, name, desired_key, new_value): @cached_property def selected_subset_group(self): for subset_group in self.app.data_collection.subset_groups: - if subset_group.label == self.selected: + if subset_group.label == self.subset.selected: return subset_group - def rename_selected(self, new_name): - if new_name in self.labels: - raise ValueError(f"{new_name} is already a named subset") - if new_name in ['Entire Spectrum', 'Surrounding']: - # these are names used in various subset dropdowns for other meanings - raise ValueError(f"{new_name} is a reserved name") + def rename_subset(self, old_label, new_label, subset_group=None): + self.app._rename_subset(old_label, new_label, subset_group) + self._sync_available_from_state() + + def rename_selected(self, new_label): + #if not self.app._check_valid_subset_label(self.selected, new_name, warn_if_invalid=False): subset_group = self.selected_subset_group if subset_group is None: raise TypeError("current selection is not a subset") + self.rename_subset(self.subset.selected, new_label, subset_group=subset_group) + def import_region(self, region, combination_mode=None, max_num_regions=None, refdata_label=None, return_bad_regions=False, **kwargs): """ diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index f07e7f878e..e7541f5564 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -49,7 +49,7 @@ ChangeRefDataMessage, PluginTableAddedMessage, PluginTableModifiedMessage, PluginPlotAddedMessage, PluginPlotModifiedMessage, - GlobalDisplayUnitChanged) + GlobalDisplayUnitChanged, SubsetRenameMessage) from jdaviz.core.marks import (LineAnalysisContinuum, LineAnalysisContinuumCenter, LineAnalysisContinuumLeft, @@ -1988,6 +1988,8 @@ def __init__(self, plugin, items, selected, multiselect=None, selected_has_subre handler=lambda msg: self._update_subset(msg.subset, msg.attribute)) self.hub.subscribe(self, SubsetDeleteMessage, handler=lambda msg: self._delete_subset(msg.subset)) + self.hub.subscribe(self, SubsetRenameMessage, + handler=lambda msg: self._rename_subset(msg)) self._initialize_choices() @@ -2073,6 +2075,28 @@ def _update_subset(self, subset, attribute=None): if self._subset_selected_changed_callback is not None: self._subset_selected_changed_callback() + def _rename_subset(self, msg): + # See if we're renaming the selected subset + update_selected = False + if self.selected == msg.old_label: + update_selected = True + + # Find the subset in self.items and update the label + for subset in self.items: + if subset['label'] == msg.old_label: + subset['label'] = msg.new_label + break + + # Update the selected label if needed + if update_selected: + self.selected = msg.new_label + + # Force the traitlet to update. This is named different things depending on the plugin + for att in ("subset_items", "aperture_items", "bg_items"): + if hasattr(self.plugin, att): + self.plugin.send_state(att) + + def _update_has_subregions(self): if "selected_has_subregions" in self._plugin_traitlets.keys(): if self.is_multiselect: From ff18e05dc8bdf7b68c4c7e989e36989f2fb39ead Mon Sep 17 00:00:00 2001 From: Kyle Conroy <kyleconroy@gmail.com> Date: Fri, 6 Dec 2024 14:44:32 -0500 Subject: [PATCH 08/37] allow plugin components to call `send_state` not needed anywhere yet --- jdaviz/core/template_mixin.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index e7541f5564..327285e1cb 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -700,6 +700,10 @@ def add_observe(self, traitlet_name, handler, first=False): new_order = [handler] + [other for other in existing_callbacks if other != handler] self._plugin._trait_notifiers[traitlet_name]['change'] = new_order + def send_state(self, traitlet_name): + # redirect send_state through the plugin + self._plugin.send_state(self._plugin_traitlets.get(traitlet_name)) + @property def plugin(self): """ From dafdd7cd9b8e234841982b1883b180e7fe60b1c5 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Fri, 13 Dec 2024 15:20:41 -0500 Subject: [PATCH 09/37] Working on updating subset items in LayerSelect on rename, buggy right now --- jdaviz/core/template_mixin.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 327285e1cb..cdb952d2cb 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -1535,6 +1535,8 @@ def __init__(self, plugin, items, selected, viewer, handler=lambda _: self._update_items()) self.hub.subscribe(self, SubsetDeleteMessage, handler=lambda _: self._update_items()) + self.hub.subscribe(self, SubsetRenameMessage, + handler=self._on_subset_renamed) self.sort_by = sort_by self.app.state.add_callback('layer_icons', self._update_items) @@ -1719,6 +1721,16 @@ def _on_subset_created(self, msg=None): # TODO: Add ability to add new item to self.items instead of recompiling self._update_items({'source': 'subset_added'}) + def _on_subset_renamed(self, msg): + print("Calling on_subset_renamed") + # Find the subset in self.items and update the label + for item in self.items: + if item['label'] == msg.old_label: + print(f"Updating {item}") + item['label'] = msg.new_label + break + self.send_state("items") + def _on_data_added(self, msg=None): if msg is None or not hasattr(msg, 'data') or msg.data is None: return @@ -1769,6 +1781,7 @@ def _update_items(self, msg={}): self.only_wcs_layers ] unique_layer_labels = list(set(layer_labels)) + print("Found layer lables: {unique_layer_labels}") layer_items = [self._layer_to_dict(layer_label) for layer_label in unique_layer_labels] def _sort_by_icon(items_dict): @@ -2095,10 +2108,12 @@ def _rename_subset(self, msg): if update_selected: self.selected = msg.new_label + # Force the traitlet to update. + self.send_state('items') # Force the traitlet to update. This is named different things depending on the plugin - for att in ("subset_items", "aperture_items", "bg_items"): - if hasattr(self.plugin, att): - self.plugin.send_state(att) + #for att in ("subset_items", "aperture_items", "bg_items"): + # if hasattr(self.plugin, att): + # self.plugin.send_state(att) def _update_has_subregions(self): From 5bf5c72808b987d138fadd2f1c0bb77a46af7e2e Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Tue, 17 Dec 2024 13:51:34 -0500 Subject: [PATCH 10/37] Add passing test, refactor app.get_subsets for better efficiency in single subset case --- jdaviz/app.py | 8 ++++--- .../plugins/subset_tools/subset_tools.py | 2 -- .../subset_tools/tests/test_subset_tools.py | 21 +++++++++++++++++++ jdaviz/core/template_mixin.py | 6 ------ 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 13d5c219d8..6cb393da82 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -979,7 +979,7 @@ def get_subsets(self, subset_name=None, spectral_only=False, for subset in subsets: - label = subset.label + label = subset.label.lower() if isinstance(subset.subset_state, CompositeSubsetState): # Region composed of multiple ROI or Range subset @@ -2030,10 +2030,11 @@ def _check_valid_subset_label(self, subset_name, warn_if_invalid=True): # now check `subset_name` against list of non-active current subset labels # and warn and return if it is + print(f"Reserved labels:\n{self._reserved_labels}") if subset_name.lower() in self._reserved_labels: if warn_if_invalid: - warnings.warn(f"Can not rename subset to name of another existing subset" - " or data item: ({subset_name}).") + warnings.warn("Cannot rename subset to name of an existing subset" + f" or data item: ({subset_name}).") return False elif not subset_name.replace(" ", "").isalnum(): @@ -2054,6 +2055,7 @@ def _rename_subset(self, old_label, new_label, subset_group=None, check_valid=Tr print(subset_group) if check_valid: if self._check_valid_subset_label(new_label): + print("Checking validity") subset_group.label = new_label else: subset_group.label = new_label diff --git a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py index 01b759d131..8e8f2e5744 100644 --- a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py @@ -859,8 +859,6 @@ def rename_subset(self, old_label, new_label, subset_group=None): self._sync_available_from_state() def rename_selected(self, new_label): - #if not self.app._check_valid_subset_label(self.selected, new_name, warn_if_invalid=False): - subset_group = self.selected_subset_group if subset_group is None: raise TypeError("current selection is not a subset") diff --git a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py index 90757df60d..e093ddcca8 100644 --- a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py @@ -315,3 +315,24 @@ def test_get_regions_composite(cubeviz_helper, spectrum1d_cube): # make sure the same regions are returned by app.get_subsets get_subsets = cubeviz_helper.app.get_subsets() assert np.all([get_subsets[ss][0]['region'] == regions[ss] for ss in ss_labels]) + + +def test_check_valid_subset_label(imviz_helper): + + # imviz instance with some data + data = NDData(np.ones((50, 50)) * u.nJy) + imviz_helper.load_data(data) + + st = imviz_helper.plugins["Subset Tools"] + + # apply three subsets, with their default names of `Subset 1`, `Subset 2`, and `Subset 3` + st.import_region(CircularROI(20, 20, 10)) + st.subset = "Create New" + st.import_region(CircularROI(25, 25, 10)) + st.subset = "Create New" + st.import_region(CircularROI(30, 30, 10)) + + # we should not be able to rename or add a subset named 'subset 1', since 'Subset 1' + # exists. make sure this warns and returns accordingly. + with pytest.warns(Warning, match="Cannot rename subset to name of an existing subset"): + st.rename_selected("subset 1") diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index cdb952d2cb..56858d742a 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -1781,7 +1781,6 @@ def _update_items(self, msg={}): self.only_wcs_layers ] unique_layer_labels = list(set(layer_labels)) - print("Found layer lables: {unique_layer_labels}") layer_items = [self._layer_to_dict(layer_label) for layer_label in unique_layer_labels] def _sort_by_icon(items_dict): @@ -2110,11 +2109,6 @@ def _rename_subset(self, msg): # Force the traitlet to update. self.send_state('items') - # Force the traitlet to update. This is named different things depending on the plugin - #for att in ("subset_items", "aperture_items", "bg_items"): - # if hasattr(self.plugin, att): - # self.plugin.send_state(att) - def _update_has_subregions(self): if "selected_has_subregions" in self._plugin_traitlets.keys(): From 9ce2c0d1297c31dcbe8ad79cd25748a0a9e9ffce Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Tue, 17 Dec 2024 15:22:46 -0500 Subject: [PATCH 11/37] Add changelog --- CHANGES.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 1bb87ddd0c..83f5d2269c 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -89,9 +89,11 @@ New Features - Improve performance while importing multiple regions. [#3321] -* API method to toggle API hints. [#3336] +- API method to toggle API hints. [#3336] -* Changing flux/SB display units no longer resets viewer zoom levels. [#3335] +- Changing flux/SB display units no longer resets viewer zoom levels. [#3335] + +- Added API for renaming subsets to Subset Tools plugin. [#3356] Cubeviz ^^^^^^^ From b1afe397ecbd7c2f1ccc0a507d2e84a32c16a240 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Wed, 18 Dec 2024 10:17:08 -0500 Subject: [PATCH 12/37] Remove leftover lower() from rebase --- jdaviz/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 6cb393da82..03a8ab3ce3 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -979,7 +979,7 @@ def get_subsets(self, subset_name=None, spectral_only=False, for subset in subsets: - label = subset.label.lower() + label = subset.label if isinstance(subset.subset_state, CompositeSubsetState): # Region composed of multiple ROI or Range subset From 7b7466fdcc4b2c0f50b45dca7c30befb4a7d5cfd Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Wed, 18 Dec 2024 11:34:11 -0500 Subject: [PATCH 13/37] Fix tests --- jdaviz/app.py | 12 +++++++----- .../plugins/subset_tools/tests/test_subset_tools.py | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 03a8ab3ce3..d65bc54a3c 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -468,11 +468,13 @@ def _on_subset_update_message(self, msg): def _on_subset_delete_message(self, msg): self._remove_live_plugin_results(trigger_subset=msg.subset) - self._reserved_labels.remove(msg.subset.label.lower()) + if msg.subset.label in self._reserved_labels: + # This might already be gone in test teardowns + self._reserved_labels.remove(msg.subset.label) self._on_layers_changed(msg) def _on_subset_create_message(self, msg): - self._reserved_labels.add(msg.subset.label.lower()) + self._reserved_labels.add(msg.subset.label) self._on_layers_changed(msg) def _on_plugin_plot_added(self, msg): @@ -2025,13 +2027,13 @@ def _check_valid_subset_label(self, subset_name, warn_if_invalid=True): # remove the current selection label from the set of labels, because its ok # if the new subset shares the name of the current selection (renaming to current name) - if subset_selected.lower() in self._reserved_labels: - self._reserved_labels.remove(subset_selected.lower()) + if subset_selected in self._reserved_labels: + self._reserved_labels.remove(subset_selected) # now check `subset_name` against list of non-active current subset labels # and warn and return if it is print(f"Reserved labels:\n{self._reserved_labels}") - if subset_name.lower() in self._reserved_labels: + if subset_name in self._reserved_labels: if warn_if_invalid: warnings.warn("Cannot rename subset to name of an existing subset" f" or data item: ({subset_name}).") diff --git a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py index e093ddcca8..e51166b30f 100644 --- a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py @@ -332,7 +332,7 @@ def test_check_valid_subset_label(imviz_helper): st.subset = "Create New" st.import_region(CircularROI(30, 30, 10)) - # we should not be able to rename or add a subset named 'subset 1', since 'Subset 1' - # exists. make sure this warns and returns accordingly. + # we should not be able to rename or add a subset named 'Subset 1'. + # Make sure this warns and returns accordingly. with pytest.warns(Warning, match="Cannot rename subset to name of an existing subset"): - st.rename_selected("subset 1") + st.rename_selected("Subset 1") From 575bdee93dd4169bc0410ddfde11a03a08479de4 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Wed, 18 Dec 2024 11:38:01 -0500 Subject: [PATCH 14/37] Remove debugging prints --- jdaviz/app.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index d65bc54a3c..be97b74168 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2032,7 +2032,6 @@ def _check_valid_subset_label(self, subset_name, warn_if_invalid=True): # now check `subset_name` against list of non-active current subset labels # and warn and return if it is - print(f"Reserved labels:\n{self._reserved_labels}") if subset_name in self._reserved_labels: if warn_if_invalid: warnings.warn("Cannot rename subset to name of an existing subset" @@ -2054,10 +2053,8 @@ def _rename_subset(self, old_label, new_label, subset_group=None, check_valid=Tr if s.label == old_label: subset_group = s break - print(subset_group) if check_valid: if self._check_valid_subset_label(new_label): - print("Checking validity") subset_group.label = new_label else: subset_group.label = new_label From 394afd083a8eee38ac9db3816c2d6ccabf88b2b4 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Thu, 19 Dec 2024 09:22:59 -0500 Subject: [PATCH 15/37] Add docstrings, remove kwarg that isn't relevant to users --- .../plugins/subset_tools/subset_tools.py | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py index 8e8f2e5744..e77e1e66d8 100644 --- a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py @@ -854,16 +854,35 @@ def selected_subset_group(self): if subset_group.label == self.subset.selected: return subset_group - def rename_subset(self, old_label, new_label, subset_group=None): - self.app._rename_subset(old_label, new_label, subset_group) + def rename_subset(self, old_label, new_label): + """ + Method to rename an existing subset + + Parameters + ---------- + old_label : str + The current label of the subset to be renamed. + new_label : str + The new label to apply to the selected subset. + """ + self.app._rename_subset(old_label, new_label) self._sync_available_from_state() def rename_selected(self, new_label): + """ + Method to rename the subset currently selected in the Subset Tools plugin. + + Parameters + ---------- + new_label : str + The new label to apply to the selected subset. + """ subset_group = self.selected_subset_group if subset_group is None: raise TypeError("current selection is not a subset") - self.rename_subset(self.subset.selected, new_label, subset_group=subset_group) + self.app._rename_subset(self.subset.selected, new_label, subset_group=subset_group) + self._sync_available_from_state() def import_region(self, region, combination_mode=None, max_num_regions=None, refdata_label=None, return_bad_regions=False, **kwargs): From 6eb51563fbe6d13c305c4d5615a630fe9a1469c8 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Thu, 19 Dec 2024 13:52:25 -0500 Subject: [PATCH 16/37] Don't allow renaming to Subset N --- jdaviz/app.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/jdaviz/app.py b/jdaviz/app.py index be97b74168..b3b6ecff9d 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2043,6 +2043,13 @@ def _check_valid_subset_label(self, subset_name, warn_if_invalid=True): warnings.warn("Subset labels must be purely alphanumeric") return False + else: + split_label = subset_name.split(" ") + if split_label[0] == "Subset" and split_label[1].isdigit(): + if warn_if_invalid: + warnings.warn("The pattern 'Subset N' is reserved for auto-generated labels") + return False + return True def _rename_subset(self, old_label, new_label, subset_group=None, check_valid=True): From 1a195ae0b0c7dbc2b671de19bb8a6a139a1f642d Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Thu, 19 Dec 2024 14:01:45 -0500 Subject: [PATCH 17/37] Raise instead of error, add test --- jdaviz/app.py | 18 +++++++++++------- .../subset_tools/tests/test_subset_tools.py | 5 ++++- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index b3b6ecff9d..f260113f1e 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2002,7 +2002,7 @@ def _get_viewer_item(self, ref_or_id): viewer_item = self._viewer_item_by_id(ref_or_id) return viewer_item - def _check_valid_subset_label(self, subset_name, warn_if_invalid=True): + def _check_valid_subset_label(self, subset_name, raise_if_invalid=True): """Check that `subset_name` is a valid choice for a subset name. This check is run when renaming subsets. @@ -2033,22 +2033,26 @@ def _check_valid_subset_label(self, subset_name, warn_if_invalid=True): # now check `subset_name` against list of non-active current subset labels # and warn and return if it is if subset_name in self._reserved_labels: - if warn_if_invalid: - warnings.warn("Cannot rename subset to name of an existing subset" + if raise_if_invalid: + raise ValueError("Cannot rename subset to name of an existing subset" f" or data item: ({subset_name}).") return False elif not subset_name.replace(" ", "").isalnum(): - if warn_if_invalid: - warnings.warn("Subset labels must be purely alphanumeric") + if raise_if_invalid: + raise ValueError("Subset labels must be purely alphanumeric") return False else: split_label = subset_name.split(" ") if split_label[0] == "Subset" and split_label[1].isdigit(): - if warn_if_invalid: - warnings.warn("The pattern 'Subset N' is reserved for auto-generated labels") + print("Got here") + if raise_if_invalid: + raise ValueError("The pattern 'Subset N' is reserved for " + "auto-generated labels") return False + else: + print("whoops") return True diff --git a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py index e51166b30f..ce0c4734fb 100644 --- a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py @@ -334,5 +334,8 @@ def test_check_valid_subset_label(imviz_helper): # we should not be able to rename or add a subset named 'Subset 1'. # Make sure this warns and returns accordingly. - with pytest.warns(Warning, match="Cannot rename subset to name of an existing subset"): + with pytest.raises(ValueError, match="Cannot rename subset to name of an existing subset"): st.rename_selected("Subset 1") + + with pytest.raises(ValueError, match="The pattern 'Subset N' is reserved"): + st.rename_selected("Subset 5") \ No newline at end of file From 2aa8afc8f82bbe061b4debaba60039bfa045622e Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Thu, 19 Dec 2024 14:03:41 -0500 Subject: [PATCH 18/37] Remove debugging prints --- jdaviz/app.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index f260113f1e..5ba240e8e6 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2046,13 +2046,10 @@ def _check_valid_subset_label(self, subset_name, raise_if_invalid=True): else: split_label = subset_name.split(" ") if split_label[0] == "Subset" and split_label[1].isdigit(): - print("Got here") if raise_if_invalid: raise ValueError("The pattern 'Subset N' is reserved for " "auto-generated labels") return False - else: - print("whoops") return True From c1c857dfaffd2b596acfe57953efafd9ad8a4d5d Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Thu, 19 Dec 2024 15:54:51 -0500 Subject: [PATCH 19/37] Raise error if trying to rename a subset that doesn't exist --- jdaviz/app.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jdaviz/app.py b/jdaviz/app.py index 5ba240e8e6..f8278c6eb3 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2061,6 +2061,10 @@ def _rename_subset(self, old_label, new_label, subset_group=None, check_valid=Tr if s.label == old_label: subset_group = s break + # If we couldn't find a matching subset group, raise an error + else: + raise ValueError(f"No subset named {old_label} to rename") + if check_valid: if self._check_valid_subset_label(new_label): subset_group.label = new_label From f579ab19ae0e35f79156f6742f2a9858798ffb1f Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Mon, 23 Dec 2024 08:59:39 -0500 Subject: [PATCH 20/37] Remove prints --- jdaviz/core/template_mixin.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 56858d742a..abf9540a86 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -1722,11 +1722,9 @@ def _on_subset_created(self, msg=None): self._update_items({'source': 'subset_added'}) def _on_subset_renamed(self, msg): - print("Calling on_subset_renamed") # Find the subset in self.items and update the label for item in self.items: if item['label'] == msg.old_label: - print(f"Updating {item}") item['label'] = msg.new_label break self.send_state("items") From ecdaf94a495722107a7dbd75610eea70481f454e Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Mon, 23 Dec 2024 11:32:18 -0500 Subject: [PATCH 21/37] codesty.e --- jdaviz/app.py | 2 +- .../default/plugins/subset_tools/tests/test_subset_tools.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index f8278c6eb3..13f97c0175 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2035,7 +2035,7 @@ def _check_valid_subset_label(self, subset_name, raise_if_invalid=True): if subset_name in self._reserved_labels: if raise_if_invalid: raise ValueError("Cannot rename subset to name of an existing subset" - f" or data item: ({subset_name}).") + f" or data item: ({subset_name}).") return False elif not subset_name.replace(" ", "").isalnum(): diff --git a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py index ce0c4734fb..3db655e150 100644 --- a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py @@ -338,4 +338,4 @@ def test_check_valid_subset_label(imviz_helper): st.rename_selected("Subset 1") with pytest.raises(ValueError, match="The pattern 'Subset N' is reserved"): - st.rename_selected("Subset 5") \ No newline at end of file + st.rename_selected("Subset 5") From 03ba4ce9bdd0355144e0b39ee883a594d4ee8528 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Tue, 24 Dec 2024 11:16:01 -0500 Subject: [PATCH 22/37] Don't check for hardcoded 'Subset' in label --- jdaviz/configs/default/plugins/subset_tools/subset_tools.py | 4 ---- jdaviz/configs/imviz/plugins/orientation/orientation.py | 4 ++-- jdaviz/core/helpers.py | 5 ----- jdaviz/core/template_mixin.py | 2 +- 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py index e77e1e66d8..3326ce55c2 100644 --- a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py @@ -241,10 +241,6 @@ def get_regions(self, region_type=None, list_of_subset_labels=None, subset_data = lyr.layer subset_label = subset_data.label - # TODO: Remove this when Jdaviz support round-tripping, see - # https://github.com/spacetelescope/jdaviz/pull/721 - if not subset_label.startswith('Subset'): - continue try: if self.app.config == "imviz" and to_sky: region = roi_subset_state_to_region(subset_data.subset_state, diff --git a/jdaviz/configs/imviz/plugins/orientation/orientation.py b/jdaviz/configs/imviz/plugins/orientation/orientation.py index 7d32a20519..de86ca981c 100644 --- a/jdaviz/configs/imviz/plugins/orientation/orientation.py +++ b/jdaviz/configs/imviz/plugins/orientation/orientation.py @@ -8,6 +8,7 @@ from glue.core.subset import Subset from glue.core.subset_group import GroupedSubset from glue.plugins.wcs_autolinking.wcs_autolinking import WCSLink, NoAffineApproximation +from glue.viewers.image.state import ImageSubsetLayerState from traitlets import List, Unicode, Bool, Dict, observe from jdaviz.configs.imviz.wcs_utils import ( @@ -501,8 +502,7 @@ def _on_refdata_change(self, msg): # ensure that it is never visible: for layer in viewer.state.layers: if ( - hasattr(layer.layer, 'label') and - layer.layer.label.startswith("Subset") and + isinstance(layer.layer, ImageSubsetLayerState) and layer.layer.data.meta.get("_WCS_ONLY", False) ): layer.visible = False diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index 94f90de2d9..44242d1a01 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -924,11 +924,6 @@ def get_interactive_regions(self): subset_data = lyr.layer subset_label = subset_data.label - # TODO: Remove this when Jdaviz support round-tripping, see - # https://github.com/spacetelescope/jdaviz/pull/721 - if not subset_label.startswith('Subset'): - continue - try: if self.app.config == "imviz" and to_sky: region = roi_subset_state_to_region(subset_data.subset_state, to_sky=to_sky) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index abf9540a86..cf490f1952 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -2068,7 +2068,7 @@ def _update_subset(self, subset, attribute=None): if subset.label not in self.labels: # NOTE: this logic will need to be revisited if generic renaming of subsets is added # see https://github.com/spacetelescope/jdaviz/pull/1175#discussion_r829372470 - if subset.label[:6] == 'Subset' and self._is_valid_item(subset): + if self._is_valid_item(subset): # NOTE: += will not trigger traitlet update self.items = self.items + [self._subset_to_dict(subset)] # noqa else: From 4e1a6946a2f278183b82669e7d409c5f0fa1312a Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Tue, 24 Dec 2024 12:25:29 -0500 Subject: [PATCH 23/37] Fix failing test --- jdaviz/configs/default/plugins/subset_tools/subset_tools.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py index 3326ce55c2..888d68d0a3 100644 --- a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py @@ -241,6 +241,10 @@ def get_regions(self, region_type=None, list_of_subset_labels=None, subset_data = lyr.layer subset_label = subset_data.label + if type(subset_data.subset_state) not in (RoiSubsetState, CompositeSubsetState): + # Ignore MaskSubsetState here + continue + try: if self.app.config == "imviz" and to_sky: region = roi_subset_state_to_region(subset_data.subset_state, From 79ee99ad85212fc13eb42fcb9591929e9b9cde9e Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Tue, 24 Dec 2024 13:07:54 -0500 Subject: [PATCH 24/37] Fix check to be more general --- jdaviz/configs/default/plugins/subset_tools/subset_tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py index 888d68d0a3..74f64dbf71 100644 --- a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py @@ -241,7 +241,7 @@ def get_regions(self, region_type=None, list_of_subset_labels=None, subset_data = lyr.layer subset_label = subset_data.label - if type(subset_data.subset_state) not in (RoiSubsetState, CompositeSubsetState): + if isinstance(subset_data.subset_state, MaskSubsetState): # Ignore MaskSubsetState here continue From 4c431a936d09ddb7168172e48a0e933a0ecb001f Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Tue, 24 Dec 2024 13:17:18 -0500 Subject: [PATCH 25/37] Move changelog entry --- CHANGES.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 83f5d2269c..b67cb9a4e6 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,6 +4,8 @@ New Features ------------ +- Added API for renaming subsets to Subset Tools plugin. [#3356] + Cubeviz ^^^^^^^ @@ -93,8 +95,6 @@ New Features - Changing flux/SB display units no longer resets viewer zoom levels. [#3335] -- Added API for renaming subsets to Subset Tools plugin. [#3356] - Cubeviz ^^^^^^^ From fe0f198cd778de0b3c59baa37ed7649576e5db33 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Thu, 26 Dec 2024 11:42:25 -0500 Subject: [PATCH 26/37] Restore updated subset to layer select menus with icon --- jdaviz/app.py | 3 +++ jdaviz/configs/default/plugins/subset_tools/subset_tools.py | 5 ----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 13f97c0175..85389b39cd 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2071,6 +2071,9 @@ def _rename_subset(self, old_label, new_label, subset_group=None, check_valid=Tr else: subset_group.label = new_label + self.state.layer_icons[new_label] = self.state.layer_icons[old_label] + _ = self.state.layer_icons.pop(old_label) + self.hub.broadcast(SubsetRenameMessage(subset_group, old_label, new_label, sender=self)) def _reparent_subsets(self, old_parent, new_parent=None): diff --git a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py index 74f64dbf71..3ec4080b47 100644 --- a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py @@ -305,11 +305,6 @@ def _sync_selected_from_state(self, *args): self._update_combination_mode() def _on_subset_update(self, msg): - ''' - if msg.attribute == "label": - print("updating label") - print(msg.subset.label) - ''' self._sync_selected_from_state() if 'Create New' in self.subset_selected: return From 5b69487a7a6c5eb02340c966a62fe1cbce6292b3 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Thu, 26 Dec 2024 12:02:04 -0500 Subject: [PATCH 27/37] Don't duplicate renamed subset in export plugin --- jdaviz/core/template_mixin.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index cf490f1952..37b9bba92d 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -2056,6 +2056,10 @@ def is_not_annulus(subset): return super()._is_valid_item(subset, locals()) def _update_subset(self, subset, attribute=None): + if attribute == 'label': + # We handle this in _rename_subset + return + if (attribute == 'subset_state' and ((self.is_multiselect and subset.label in self.selected) or (subset.label == self.selected))): From 70d4058430599b4a95a5559bad6ef7e9e0e082a6 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Thu, 26 Dec 2024 14:39:06 -0500 Subject: [PATCH 28/37] Update extracted spectrum data collection item as well Update extracted spectrum data collection item as well --- jdaviz/app.py | 15 +++++++++++++++ jdaviz/core/template_mixin.py | 1 + 2 files changed, 16 insertions(+) diff --git a/jdaviz/app.py b/jdaviz/app.py index 85389b39cd..30e4a5af35 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2071,9 +2071,24 @@ def _rename_subset(self, old_label, new_label, subset_group=None, check_valid=Tr else: subset_group.label = new_label + # Update layer icon self.state.layer_icons[new_label] = self.state.layer_icons[old_label] _ = self.state.layer_icons.pop(old_label) + # Updated extracted spectrum if applicable and update _update_live_plugin_results dict. + if self.config == 'cubeviz': + for d in self.data_collection: + if d.label.split("(")[-1].split(",")[0] == old_label: + old_data_label = d.label + new_data_label = d.label.replace(old_label, new_label) + d.label = new_data_label + self.state.layer_icons[new_data_label] = self.state.layer_icons[old_data_label] + _ = self.state.layer_icons.pop(old_data_label) + results_dict = d.meta['_update_live_plugin_results'] + results_dict['aperture'] = new_label + results_dict['add_results']['label'] = new_data_label + d.meta['_update_live_plugin_results'] = results_dict + self.hub.broadcast(SubsetRenameMessage(subset_group, old_label, new_label, sender=self)) def _reparent_subsets(self, old_parent, new_parent=None): diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 37b9bba92d..5dfc1a4b23 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -3551,6 +3551,7 @@ def __init__(self, plugin, items, selected, self.hub.subscribe(self, RemoveDataMessage, handler=self._update_items) self.hub.subscribe(self, DataCollectionAddMessage, handler=self._update_items) self.hub.subscribe(self, DataCollectionDeleteMessage, handler=self._update_items) + self.hub.subscribe(self, SubsetRenameMessage, handler=self._update_items) self.hub.subscribe(self, GlobalDisplayUnitChanged, handler=self._on_global_display_unit_changed) From 5e4b4248d510abfde35ecda0c99accdc5d5cfdde Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Thu, 26 Dec 2024 14:54:48 -0500 Subject: [PATCH 29/37] Update old data menu items --- jdaviz/app.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jdaviz/app.py b/jdaviz/app.py index 30e4a5af35..699050db1e 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2089,6 +2089,10 @@ def _rename_subset(self, old_label, new_label, subset_group=None, check_valid=Tr results_dict['add_results']['label'] = new_data_label d.meta['_update_live_plugin_results'] = results_dict + for data_item in self.state.data_items: + if data_item['name'] == old_data_label: + data_item['name'] = new_data_label + self.hub.broadcast(SubsetRenameMessage(subset_group, old_label, new_label, sender=self)) def _reparent_subsets(self, old_parent, new_parent=None): From 2f35bc1cb8f83552496ec68eb827a0289bdc9fde Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Fri, 27 Dec 2024 10:17:42 -0500 Subject: [PATCH 30/37] Address review comments --- jdaviz/app.py | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 699050db1e..2f98dc9756 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2075,23 +2075,32 @@ def _rename_subset(self, old_label, new_label, subset_group=None, check_valid=Tr self.state.layer_icons[new_label] = self.state.layer_icons[old_label] _ = self.state.layer_icons.pop(old_label) - # Updated extracted spectrum if applicable and update _update_live_plugin_results dict. - if self.config == 'cubeviz': - for d in self.data_collection: - if d.label.split("(")[-1].split(",")[0] == old_label: - old_data_label = d.label - new_data_label = d.label.replace(old_label, new_label) - d.label = new_data_label - self.state.layer_icons[new_data_label] = self.state.layer_icons[old_data_label] - _ = self.state.layer_icons.pop(old_data_label) - results_dict = d.meta['_update_live_plugin_results'] - results_dict['aperture'] = new_label + # Updated derived data if applicable + for d in self.data_collection: + data_renamed = False + # Extracted spectra are named, e.g., 'Data (Subset 1, sum)' + if d.label.split("(")[-1].split(",")[0] == old_label: + old_data_label = d.label + new_data_label = d.label.replace(old_label, new_label) + d.label = new_data_label + self.state.layer_icons[new_data_label] = self.state.layer_icons[old_data_label] + _ = self.state.layer_icons.pop(old_data_label) + + # Update the entries in the old data menu + for data_item in self.state.data_items: + if data_item['name'] == old_data_label: + data_item['name'] = new_data_label + + # Update live plugin results subscriptions + if hasattr(d, 'meta') and '_update_live_plugin_results' in d.meta: + results_dict = d.meta['_update_live_plugin_results'] + for key in results_dict.get('_subscriptions', {}).get('subset'): + results_dict[key] = new_label + + if data_renamed: results_dict['add_results']['label'] = new_data_label - d.meta['_update_live_plugin_results'] = results_dict - for data_item in self.state.data_items: - if data_item['name'] == old_data_label: - data_item['name'] = new_data_label + d.meta['_update_live_plugin_results'] = results_dict self.hub.broadcast(SubsetRenameMessage(subset_group, old_label, new_label, sender=self)) From 1f1978e9e3d0673890cb7423624e39f2cd8bb724 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Fri, 27 Dec 2024 10:32:10 -0500 Subject: [PATCH 31/37] Codestyle --- jdaviz/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 2f98dc9756..89e9f427b6 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2095,7 +2095,7 @@ def _rename_subset(self, old_label, new_label, subset_group=None, check_valid=Tr if hasattr(d, 'meta') and '_update_live_plugin_results' in d.meta: results_dict = d.meta['_update_live_plugin_results'] for key in results_dict.get('_subscriptions', {}).get('subset'): - results_dict[key] = new_label + results_dict[key] = new_label if data_renamed: results_dict['add_results']['label'] = new_data_label From cbc0698556cfaf75a6d09f0812c0f4ffeeb1112e Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Fri, 27 Dec 2024 10:34:28 -0500 Subject: [PATCH 32/37] Check old label --- jdaviz/app.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 89e9f427b6..ffbd64a19d 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -2095,7 +2095,8 @@ def _rename_subset(self, old_label, new_label, subset_group=None, check_valid=Tr if hasattr(d, 'meta') and '_update_live_plugin_results' in d.meta: results_dict = d.meta['_update_live_plugin_results'] for key in results_dict.get('_subscriptions', {}).get('subset'): - results_dict[key] = new_label + if results_dict[key] == old_label: + results_dict[key] = new_label if data_renamed: results_dict['add_results']['label'] = new_data_label From c64656eacb04f745c75315d4fbda4fd45e163a7c Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Fri, 27 Dec 2024 11:18:23 -0500 Subject: [PATCH 33/37] Add simple test --- .../subset_tools/tests/test_subset_tools.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py index 3db655e150..bbbbef04bd 100644 --- a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py @@ -339,3 +339,20 @@ def test_check_valid_subset_label(imviz_helper): with pytest.raises(ValueError, match="The pattern 'Subset N' is reserved"): st.rename_selected("Subset 5") + + +def test_rename_subset(cubeviz_helper, spectrum1d_cube): + + cubeviz_helper.load_data(spectrum1d_cube) + plg = cubeviz_helper.plugins['Subset Tools'] + + spatial_reg = CirclePixelRegion(center=PixCoord(x=2, y=2), radius=2) + plg.import_region(spatial_reg, combination_mode='new') + spatial_reg = CirclePixelRegion(center=PixCoord(x=4, y=4), radius=1) + plg.import_region(spatial_reg, combination_mode='new') + + plg.rename_subset("Subset 1", "Test Rename") + + print(cubeviz_helper.app.data_collection) + assert plg.subset.choices == ['Create New', 'Test Rename', 'Subset 2'] + assert cubeviz_helper.app.data_collection[-1].label == "Spectrum (Test Rename, sum)" \ No newline at end of file From cf79558ce6eefbc4249009690f67507e10302d0b Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Fri, 27 Dec 2024 11:19:23 -0500 Subject: [PATCH 34/37] Test rename_selected --- .../default/plugins/subset_tools/tests/test_subset_tools.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py index bbbbef04bd..c2c86a009c 100644 --- a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py @@ -355,4 +355,7 @@ def test_rename_subset(cubeviz_helper, spectrum1d_cube): print(cubeviz_helper.app.data_collection) assert plg.subset.choices == ['Create New', 'Test Rename', 'Subset 2'] - assert cubeviz_helper.app.data_collection[-1].label == "Spectrum (Test Rename, sum)" \ No newline at end of file + assert cubeviz_helper.app.data_collection[-1].label == "Spectrum (Test Rename, sum)" + + plg.rename_selected("Second Test") + assert plg.subset.choices == ['Create New', 'Test Rename', 'Second Test'] \ No newline at end of file From 8e95d39d9fe2b26aea78094cb6c0645f5e8a6998 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Fri, 27 Dec 2024 11:19:57 -0500 Subject: [PATCH 35/37] Codestyle --- .../default/plugins/subset_tools/tests/test_subset_tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py index c2c86a009c..c0cf7067ab 100644 --- a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py @@ -358,4 +358,4 @@ def test_rename_subset(cubeviz_helper, spectrum1d_cube): assert cubeviz_helper.app.data_collection[-1].label == "Spectrum (Test Rename, sum)" plg.rename_selected("Second Test") - assert plg.subset.choices == ['Create New', 'Test Rename', 'Second Test'] \ No newline at end of file + assert plg.subset.choices == ['Create New', 'Test Rename', 'Second Test'] From 851f20096f537f83932e91e285184860487a63f6 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Fri, 27 Dec 2024 12:12:11 -0500 Subject: [PATCH 36/37] Cover another case --- .../default/plugins/subset_tools/tests/test_subset_tools.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py index c0cf7067ab..894409931a 100644 --- a/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py @@ -359,3 +359,6 @@ def test_rename_subset(cubeviz_helper, spectrum1d_cube): plg.rename_selected("Second Test") assert plg.subset.choices == ['Create New', 'Test Rename', 'Second Test'] + + with pytest.raises(ValueError, match="No subset named BadLabel to rename"): + plg.rename_subset("BadLabel", "Failure") From 0d742756620a80f0de51a20bb5e71510470ca234 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <rosteen@stsci.edu> Date: Mon, 30 Dec 2024 09:04:21 -0500 Subject: [PATCH 37/37] Add methods to class docstring --- jdaviz/configs/default/plugins/subset_tools/subset_tools.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py index 3ec4080b47..81b6750613 100644 --- a/jdaviz/configs/default/plugins/subset_tools/subset_tools.py +++ b/jdaviz/configs/default/plugins/subset_tools/subset_tools.py @@ -85,6 +85,8 @@ class SubsetTools(PluginTemplateMixin): * :meth:`set_center` * :meth:`import_region` * :meth:`get_regions` + * :meth:`rename_selected` + * :meth:`rename_subset` """ template_file = __file__, "subset_tools.vue" select = List([]).tag(sync=True)