Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

data-menu: handle viewer rename properly #3383

Merged
merged 2 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ Other Changes and Additions
Bug Fixes
---------

- Fixes traceback from the data-menu that can be caused by a viewer rename. [#3383]

Cubeviz
^^^^^^^

Expand Down
9 changes: 8 additions & 1 deletion jdaviz/configs/default/plugins/data_menu/data_menu.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
from jdaviz.core.template_mixin import (TemplateMixin, LayerSelect,
LayerSelectMixin, DatasetSelectMixin)
from jdaviz.core.user_api import UserApiWrapper
from jdaviz.core.events import IconsUpdatedMessage, AddDataMessage, ChangeRefDataMessage
from jdaviz.core.events import (IconsUpdatedMessage, AddDataMessage,
ChangeRefDataMessage, ViewerRenamedMessage)
from jdaviz.utils import cmap_samples, is_not_wcs_only

from glue.core.edit_subset_mode import (AndMode, AndNotMode, OrMode,
Expand Down Expand Up @@ -132,6 +133,7 @@
self.hub.subscribe(self, IconsUpdatedMessage, self._on_app_icons_updated)
self.hub.subscribe(self, AddDataMessage, handler=lambda _: self._set_viewer_id())
self.hub.subscribe(self, ChangeRefDataMessage, handler=self._on_refdata_change)
self.hub.subscribe(self, ViewerRenamedMessage, handler=self._on_viewer_renamed_message)
self.viewer_icons = dict(self.app.state.viewer_icons)
self.layer_icons = dict(self.app.state.layer_icons)

Expand Down Expand Up @@ -210,6 +212,11 @@
with self.during_select_sync():
self.orientation.selected = str(self._viewer.state.reference_data.label)

def _on_viewer_renamed_message(self, msg):
if self.viewer_reference == msg.old_viewer_ref:
self.viewer_reference = msg.new_viewer_ref
self._set_viewer_id()

Check warning on line 218 in jdaviz/configs/default/plugins/data_menu/data_menu.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/default/plugins/data_menu/data_menu.py#L217-L218

Added lines #L217 - L218 were not covered by tests

@observe('orientation_layer_selected')
def _orientation_layer_selected_changed(self, event={}):
if not hasattr(self, 'orientation'):
Expand Down
10 changes: 10 additions & 0 deletions jdaviz/core/template_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1537,6 +1537,8 @@
handler=lambda _: self._update_items())
self.hub.subscribe(self, SubsetRenameMessage,
handler=self._on_subset_renamed)
self.hub.subscribe(self, ViewerRenamedMessage,
self._on_viewer_renamed_message)

self.sort_by = sort_by
self.app.state.add_callback('layer_icons', self._update_items)
Expand Down Expand Up @@ -1575,6 +1577,14 @@

self.add_filter(filter_has_children)

def _on_viewer_renamed_message(self, msg):
if isinstance(self.viewer, list):
for i, viewer in self.viewer:
if viewer == msg.old_viewer_ref:
self.viewer[i] = msg.new_viewer_ref

Check warning on line 1584 in jdaviz/core/template_mixin.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/core/template_mixin.py#L1582-L1584

Added lines #L1582 - L1584 were not covered by tests
elif self.viewer == msg.old_viewer_ref:
self.viewer = msg.new_viewer_ref

Check warning on line 1586 in jdaviz/core/template_mixin.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/core/template_mixin.py#L1586

Added line #L1586 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this need to set viewer id as well like in data_menu?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, this only tracks the reference so doesn't need to worry about whether the id has changed or not


def _get_viewer(self, viewer):
# newer will likely be the viewer name in most cases, but viewer id in the case
# of additional viewers in imviz.
Expand Down
Loading