Skip to content

Commit

Permalink
alternate upstream renaming implementation (compare with #35)
Browse files Browse the repository at this point in the history
  • Loading branch information
bmorris3 committed Sep 22, 2023
1 parent 146d0d0 commit 14d6e27
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 81 deletions.
11 changes: 1 addition & 10 deletions lcviz/events.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,9 @@
from glue.core.message import Message

__all__ = ['ViewerRenamedMessage', 'EphemerisComponentChangedMessage',
__all__ = ['EphemerisComponentChangedMessage',
'EphemerisChangedMessage']


class ViewerRenamedMessage(Message):
"""Message emitted after a viewer is destroyed by the application."""
def __init__(self, old_viewer_ref, new_viewer_ref, *args, **kwargs):
super().__init__(*args, **kwargs)

self.old_viewer_ref = old_viewer_ref
self.new_viewer_ref = new_viewer_ref


class EphemerisComponentChangedMessage(Message):
"""Message emitted when an ephemeris component is added/renamed/removed in the
ephemeris plugin"""
Expand Down
46 changes: 0 additions & 46 deletions lcviz/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from glue.core.component_id import ComponentID
from glue.core.link_helpers import LinkSame
from jdaviz.core.helpers import ConfigHelper
from lcviz.events import ViewerRenamedMessage

__all__ = ['LCviz']

Expand Down Expand Up @@ -47,46 +46,6 @@ def _get_range_subset_bounds(self, subset_state, *args, **kwargs):
"subset_state": subset_state}]


def _rename_viewer(app, old_reference, new_reference):
"""
Rename a viewer. If the ID and reference match, the ID will also be updated,
otherwise it will be kept.
CAUTION: use with caution as this does not currently update default
viewer reference names stored in helpers and could break behavior.
Parameters
----------
old_reference : str
The current reference of the viewer
new_reference : str
The desired new reference name of the viewer
"""
self = app
if new_reference in self.get_viewer_reference_names():
raise ValueError(f"viewer with reference='{new_reference}' already exists")
if new_reference in self.get_viewer_ids():
raise ValueError(f"viewer with id='{new_reference}' already exists")

viewer_item = self._get_viewer_item(old_reference)
old_id = viewer_item['id']

self._viewer_store[old_id]._reference_id = new_reference

viewer_item['reference'] = new_reference

if viewer_item['name'] == old_reference:
viewer_item['name'] = new_reference

if viewer_item['id'] == old_reference:
# update the id as well
viewer_item['id'] = new_reference
self._viewer_store[new_reference] = self._viewer_store.pop(old_id)
self.state.viewer_icons[new_reference] = self.state.viewer_icons.pop(old_id)

self.hub.broadcast(ViewerRenamedMessage(old_reference, new_reference, sender=self))


def _link_new_data(app, reference_data=None, data_to_be_linked=None):
dc = app.data_collection
ref_data = dc[reference_data] if reference_data else dc[0]
Expand Down Expand Up @@ -135,11 +94,6 @@ def __init__(self, *args, **kwargs):
lambda *args, **kwargs: _get_range_subset_bounds(self.app, *args, **kwargs)
)

# TODO: remove this if/when jdaviz supports renaming viewers natively
self.app._rename_viewer = (
lambda *args, **kwargs: _rename_viewer(self.app, *args, **kwargs)
)

self.app._link_new_data = (
lambda *args, **kwargs: _link_new_data(self.app, *args, **kwargs)
)
Expand Down
6 changes: 3 additions & 3 deletions lcviz/plugins/binning/binning.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,13 @@ def _live_update(self, event={}):
times = lc_time

for viewer_id, mark in self.marks.items():
viewer_ref = self.app.get_viewer_by_id(viewer_id).reference
if self.ephemeris_selected == 'No ephemeris':
visible = True
# TODO: fix this to be general and not rely on ugly id
do_phase = viewer_id != 'lcviz-0'
do_phase = viewer_ref != 'flux-vs-time'
else:
# TODO: try to fix flashing as traitlets update
visible = viewer_id.split(':')[-1] == self.ephemeris_selected
visible = viewer_ref.split(':')[-1] == self.ephemeris_selected
do_phase = False

if visible:
Expand Down
5 changes: 0 additions & 5 deletions lcviz/plugins/coords_info/coords_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from jdaviz.core.registries import tool_registry

from lcviz.viewers import TimeScatterView, PhaseScatterView
from lcviz.events import ViewerRenamedMessage


__all__ = ['CoordsInfo']
Expand All @@ -19,10 +18,6 @@ class CoordsInfo(CoordsInfo):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

# TODO: move to jdaviz if/once viewer renaming supported
self.hub.subscribe(self, ViewerRenamedMessage,
handler=self._viewer_renamed)

def _viewer_renamed(self, msg):
self._marks[msg.new_viewer_ref] = self._marks.pop(msg.old_viewer_ref)

Expand Down
9 changes: 6 additions & 3 deletions lcviz/plugins/ephemeris/ephemeris.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,11 @@ def _on_component_add(self, lbl):
def _on_component_rename(self, old_lbl, new_lbl):
# this is triggered when the plugin component detects a change to the component name
self._ephemerides[new_lbl] = self._ephemerides.pop(old_lbl, {})
if self._phase_viewer_id(old_lbl) in self.app.get_viewer_ids():
self.app._rename_viewer(self._phase_viewer_id(old_lbl), self._phase_viewer_id(new_lbl))
if self._phase_viewer_id(old_lbl) in self.app.get_viewer_reference_names():
self.app._update_viewer_reference_name(
self._phase_viewer_id(old_lbl),
self._phase_viewer_id(new_lbl)
)

# update metadata entries so that they can be used for filtering applicable entries in
# data menus
Expand All @@ -336,7 +339,7 @@ def _on_component_rename(self, old_lbl, new_lbl):
def _on_component_remove(self, lbl):
_ = self._ephemerides.pop(lbl, {})
# remove the corresponding viewer, if it exists
viewer_item = self.app._viewer_item_by_id(self._phase_viewer_id(lbl))
viewer_item = self.app._viewer_item_by_reference(self._phase_viewer_id(lbl))
if viewer_item is None: # pragma: no cover
return
cid = viewer_item.get('id', None)
Expand Down
16 changes: 2 additions & 14 deletions lcviz/template_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,12 @@
from ipyvuetify import VuetifyTemplate
from glue.core import HubListener

import jdaviz
from jdaviz.core.template_mixin import ViewerSelect, DatasetSelect, SelectPluginComponent
from lcviz.events import ViewerRenamedMessage, EphemerisComponentChangedMessage
from jdaviz.core.template_mixin import DatasetSelect, SelectPluginComponent
from lcviz.events import EphemerisComponentChangedMessage

__all__ = ['EphemerisSelect', 'EphemerisSelectMixin']


# TODO: remove this if/when jdaviz supports renaming viewers natively
class ViewerSelect(ViewerSelect):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.hub.subscribe(self, ViewerRenamedMessage, handler=self._on_viewers_changed)


# monkey-patch upstream version so all plugins use the viewer-renamed logic
jdaviz.core.template_mixin.ViewerSelect = ViewerSelect


class EphemerisSelect(SelectPluginComponent):
"""
Plugin select for ephemeris components defined by the Ephemeris plugin.
Expand Down

0 comments on commit 14d6e27

Please sign in to comment.