From db3548d4e339cf25852237ceca1e560b7ed99323 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Mon, 26 Feb 2024 09:55:27 -0500 Subject: [PATCH 1/2] Fix cloned phase-viewers when deleting/renaming an ephemeris component (#91) * regression test * fix ephemeris component rename/delete with a cloned phase viewer * add to existing changelog entry --- CHANGES.rst | 2 +- lcviz/plugins/ephemeris/ephemeris.py | 34 ++++++++++++++++------------ lcviz/tests/test_plugin_ephemeris.py | 21 ++++++++++++++++- 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 330d9cf6..9828ff67 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,7 +1,7 @@ 0.2.0 - unreleased ------------------ -* Clone viewer tool. [#74] +* Clone viewer tool. [#74, #91] * Flux column plugin to choose which column is treated as the flux column for each dataset. [#77] diff --git a/lcviz/plugins/ephemeris/ephemeris.py b/lcviz/plugins/ephemeris/ephemeris.py index 1f509168..b6b62f59 100644 --- a/lcviz/plugins/ephemeris/ephemeris.py +++ b/lcviz/plugins/ephemeris/ephemeris.py @@ -155,10 +155,19 @@ def phase_viewer_id(self): return self._phase_viewer_id(self.component_selected) @property - def phase_viewer(self): + def default_phase_viewer(self): if not self.phase_viewer_exists: return None - return self.app.get_viewer(self.phase_viewer_id) + # we'll just treat the "default" as the first viewer connected to this + # ephemeris component + return self._get_phase_viewers()[0] + + def _get_phase_viewers(self, lbl=None): + if lbl is None: + lbl = self.component_selected + return [viewer for vid, viewer in self.app._viewer_store.items() + if isinstance(viewer, PhaseScatterView) + and viewer.ephemeris_component == lbl] @property def ephemerides(self): @@ -312,8 +321,7 @@ def vue_period_double(self, *args): self.period *= 2 def _check_if_phase_viewer_exists(self, *args): - viewer_base_refs = [id.split('[')[0] for id in self.app.get_viewer_ids()] - self.phase_viewer_exists = self.phase_viewer_id in viewer_base_refs + self.phase_viewer_exists = len(self._get_phase_viewers()) > 0 def _validate_component(self, lbl): if '[' in lbl or ']' in lbl: @@ -329,10 +337,10 @@ 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(): + for viewer in self._get_phase_viewers(old_lbl): self.app._update_viewer_reference_name( - self._phase_viewer_id(old_lbl), - self._phase_viewer_id(new_lbl), + viewer._ref_or_id, + viewer._ref_or_id.replace(old_lbl, new_lbl), update_id=True ) @@ -351,13 +359,9 @@ 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)) - if viewer_item is None: # pragma: no cover - return - cid = viewer_item.get('id', None) - if cid is not None: - self.app.vue_destroy_viewer_item(cid) + # remove the corresponding viewer(s), if any exist + for viewer in self._get_phase_viewers(lbl): + self.app.vue_destroy_viewer_item(viewer._ref_or_id) self.hub.broadcast(EphemerisComponentChangedMessage(old_lbl=lbl, new_lbl=None, sender=self)) @@ -467,7 +471,7 @@ def round_to_1(x): if event.get('name') == 'wrap_at': old = event.get('old') if event.get('old') != '' else self._prev_wrap_at if event.get('new') != '': - pvs = self.phase_viewer.state + pvs = self.default_phase_viewer.state delta_phase = event.get('new') - old pvs.x_min, pvs.x_max = pvs.x_min + delta_phase, pvs.x_max + delta_phase # we need to cache the old value since it could become a string diff --git a/lcviz/tests/test_plugin_ephemeris.py b/lcviz/tests/test_plugin_ephemeris.py index d5e6cac9..6fd7d69e 100644 --- a/lcviz/tests/test_plugin_ephemeris.py +++ b/lcviz/tests/test_plugin_ephemeris.py @@ -36,7 +36,7 @@ def test_plugin_ephemeris(helper, light_curve_like_kepler_quarter): ephem._obj.vue_period_halve() assert ephem.period == 3.14 - pv = ephem._obj.phase_viewer + pv = ephem._obj.default_phase_viewer # original limits are set to 0->1 (technically 1-phase_wrap -> phase_wrap) assert (pv.state.x_min, pv.state.x_max) == (0.0, 1.0) ephem.wrap_at = 0.5 @@ -84,3 +84,22 @@ def test_plugin_ephemeris(helper, light_curve_like_kepler_quarter): # test that non-zero dpdt does not crash ephem.dpdt = 0.005 + + +def test_cloned_phase_viewer(helper, light_curve_like_kepler_quarter): + helper.load_data(light_curve_like_kepler_quarter) + ephem = helper.plugins['Ephemeris'] + + pv1 = ephem.create_phase_viewer() + pv2 = pv1._obj.clone_viewer() + assert len(helper.viewers) == 3 + assert pv1._obj.reference_id == 'flux-vs-phase:default' + assert pv2._obj.reference_id == 'flux-vs-phase:default[1]' + + # renaming ephemeris should update both labels + ephem.rename_component('default', 'renamed') + assert pv1._obj.reference_id == 'flux-vs-phase:renamed' + assert pv2._obj.reference_id == 'flux-vs-phase:renamed[1]' + + ephem.remove_component('renamed') + assert len(helper.viewers) == 1 # just flux-vs-phase From 7b78df59f8075a73fcb66a9aa08e1a0a52db92cf Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Mon, 11 Mar 2024 10:23:11 -0400 Subject: [PATCH 2/2] Viewer creator in app toolbar (#94) --- CHANGES.rst | 7 +- lcviz/helper.py | 45 ++++++-- lcviz/parsers.py | 15 ++- lcviz/plugins/__init__.py | 4 +- lcviz/plugins/binning/binning.py | 34 +++--- lcviz/plugins/ephemeris/ephemeris.py | 106 ++++++++++-------- lcviz/plugins/flatten/flatten.py | 10 +- lcviz/plugins/viewer_creator/__init__.py | 1 + .../plugins/viewer_creator/viewer_creator.py | 48 ++++++++ lcviz/tests/test_parser.py | 4 +- lcviz/tests/test_plugin_binning.py | 4 +- lcviz/tests/test_plugin_ephemeris.py | 36 +++++- lcviz/tests/test_plugin_flatten.py | 2 +- lcviz/tests/test_plugin_frequency_analysis.py | 1 - lcviz/tests/test_plugin_markers.py | 2 +- lcviz/tests/test_translator.py | 4 +- lcviz/tests/test_tray_viewer_creator.py | 9 ++ lcviz/tests/test_viewers.py | 6 +- lcviz/viewers.py | 27 ++--- 19 files changed, 250 insertions(+), 115 deletions(-) create mode 100644 lcviz/plugins/viewer_creator/__init__.py create mode 100644 lcviz/plugins/viewer_creator/viewer_creator.py create mode 100644 lcviz/tests/test_tray_viewer_creator.py diff --git a/CHANGES.rst b/CHANGES.rst index 9828ff67..dc07f98f 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,4 +1,9 @@ -0.2.0 - unreleased +0.3.0 - unreleased +------------------ + +* Ability to create additional viewers. [#94] + +0.2.0 (02-26-2024) ------------------ * Clone viewer tool. [#74, #91] diff --git a/lcviz/helper.py b/lcviz/helper.py index 9f234d37..295d4477 100644 --- a/lcviz/helper.py +++ b/lcviz/helper.py @@ -7,11 +7,10 @@ from glue.core.component_id import ComponentID from glue.core.link_helpers import LinkSame from jdaviz.core.helpers import ConfigHelper +from lcviz.viewers import TimeScatterView __all__ = ['LCviz'] -_default_time_viewer_reference_name = 'flux-vs-time' - custom_components = {'plugin-ephemeris-select': 'components/plugin_ephemeris_select.vue'} # Register pure vue component. This allows us to do recursive component instantiation only in the @@ -22,11 +21,7 @@ def _get_range_subset_bounds(self, subset_state, *args, **kwargs): - # Instead of overriding the jdaviz version of this method on jdaviz.Application, - # we could put in jdaviz by (1) checking if helper has a - # _default_time_viewer_reference_name, (2) using the LCviz version if so, and (3) - # using the jdaviz version otherwise. - viewer = self.get_viewer(self._jdaviz_helper._default_time_viewer_reference_name) + viewer = self._jdaviz_helper.default_time_viewer._obj light_curve = viewer.data()[0] reference_time = light_curve.meta['reference_time'] if viewer: @@ -71,7 +66,7 @@ class LCviz(ConfigHelper): 'tab_headers': True}, 'dense_toolbar': False, 'context': {'notebook': {'max_height': '600px'}}}, - 'toolbar': ['g-data-tools', 'g-subset-tools', 'lcviz-coords-info'], + 'toolbar': ['g-data-tools', 'g-subset-tools', 'lcviz-viewer-creator', 'lcviz-coords-info'], 'tray': ['lcviz-metadata-viewer', 'flux-column', 'lcviz-plot-options', 'lcviz-subset-plugin', 'lcviz-markers', 'flatten', 'frequency-analysis', 'ephemeris', @@ -86,7 +81,6 @@ class LCviz(ConfigHelper): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self._default_time_viewer_reference_name = _default_time_viewer_reference_name # override jdaviz behavior to support temporal subsets self.app._get_range_subset_bounds = ( @@ -152,6 +146,39 @@ def get_data(self, data_label=None, cls=LightCurve, subset=None): """ return super()._get_data(data_label=data_label, mask_subset=subset, cls=cls) + @property + def default_time_viewer(self): + tvs = [viewer for vid, viewer in self.app._viewer_store.items() + if isinstance(viewer, TimeScatterView)] + if not len(tvs): + raise ValueError("no time viewers exist") + return tvs[0].user_api + + @property + def _tray_tools(self): + """ + Access API objects for plugins in the app toolbar. + + Returns + ------- + plugins : dict + dict of plugin objects + """ + # TODO: provide user-friendly labels, user API, and move upstream to be public + # for now this is just useful for dev-debugging access to toolbar entries + from ipywidgets.widgets import widget_serialization + return {item['name']: widget_serialization['from_json'](item['widget'], None) + for item in self.app.state.tool_items} + + def _get_clone_viewer_reference(self, reference): + base_name = reference.split("[")[0] + name = base_name + ind = 0 + while name in self.viewers.keys(): + ind += 1 + name = f"{base_name}[{ind}]" + return name + def _phase_comp_lbl(self, component): return f'phase:{component}' diff --git a/lcviz/parsers.py b/lcviz/parsers.py index c7d20dea..70d8673d 100644 --- a/lcviz/parsers.py +++ b/lcviz/parsers.py @@ -3,13 +3,13 @@ from jdaviz.core.registries import data_parser_registry import lightkurve +from lcviz.viewers import PhaseScatterView, TimeScatterView + __all__ = ["light_curve_parser"] @data_parser_registry("light_curve_parser") def light_curve_parser(app, file_obj, data_label=None, show_in_viewer=True, **kwargs): - time_viewer_reference_name = app._jdaviz_helper._default_time_viewer_reference_name - # load local FITS file from disk by its path: if isinstance(file_obj, str) and os.path.exists(file_obj): if data_label is None: @@ -42,12 +42,11 @@ def light_curve_parser(app, file_obj, data_label=None, show_in_viewer=True, **kw app.add_data(data, new_data_label) if show_in_viewer: - app.add_data_to_viewer(time_viewer_reference_name, new_data_label) - - # add to any known phase viewers - ephem_plugin = app._jdaviz_helper.plugins.get('Ephemeris', None) - if ephem_plugin is not None: - for viewer_id in ephem_plugin._obj.phase_viewer_ids: + # add to any known time/phase viewers + for viewer_id, viewer in app._viewer_store.items(): + if isinstance(viewer, TimeScatterView): + app.add_data_to_viewer(viewer_id, new_data_label) + elif isinstance(viewer, PhaseScatterView): app.add_data_to_viewer(viewer_id, new_data_label) diff --git a/lcviz/plugins/__init__.py b/lcviz/plugins/__init__.py index 2bf109b5..42ba1f17 100644 --- a/lcviz/plugins/__init__.py +++ b/lcviz/plugins/__init__.py @@ -1,5 +1,7 @@ -from .binning.binning import * # noqa +from .viewer_creator.viewer_creator import * # noqa from .coords_info.coords_info import * # noqa + +from .binning.binning import * # noqa from .ephemeris.ephemeris import * # noqa from .export_plot.export_plot import * # noqa from .flatten.flatten import * # noqa diff --git a/lcviz/plugins/binning/binning.py b/lcviz/plugins/binning/binning.py index 97384eb5..f2440404 100644 --- a/lcviz/plugins/binning/binning.py +++ b/lcviz/plugins/binning/binning.py @@ -15,9 +15,9 @@ from lcviz.components import FluxColumnSelectMixin from lcviz.events import EphemerisChangedMessage -from lcviz.helper import _default_time_viewer_reference_name from lcviz.marks import LivePreviewBinning from lcviz.parsers import _data_with_reftime +from lcviz.viewers import TimeScatterView, PhaseScatterView from lcviz.components import EphemerisSelectMixin @@ -68,7 +68,8 @@ def not_from_binning_plugin(data): return data.meta.get('Plugin', None) != self.__class__.__name__ self.dataset.add_filter(not_from_binning_plugin) - self.hub.subscribe(self, ViewerAddedMessage, handler=self._set_results_viewer) + # TODO: viewer added also needs to repopulate marks + self.hub.subscribe(self, ViewerAddedMessage, handler=self._on_add_viewer) self.hub.subscribe(self, ViewerRemovedMessage, handler=self._set_results_viewer) self.hub.subscribe(self, EphemerisChangedMessage, handler=self._on_ephemeris_update) @@ -95,15 +96,15 @@ def input_lc(self): @property def marks(self): marks = {} - for id, viewer in self.app._viewer_store.items(): + for viewer in self.app._viewer_store.values(): for mark in viewer.figure.marks: if isinstance(mark, LivePreviewBinning): - marks[id] = mark + marks[viewer.reference] = mark break else: mark = LivePreviewBinning(viewer, visible=self.is_active) viewer.figure.marks = viewer.figure.marks + [mark] - marks[id] = mark + marks[viewer.reference] = mark return marks def _clear_marks(self): @@ -129,25 +130,31 @@ def _set_results_viewer(self, event={}): def viewer_filter(viewer): if self.ephemeris_selected in self.ephemeris._manual_options: - return viewer.reference == _default_time_viewer_reference_name - if 'flux-vs-phase:' not in viewer.reference: + return isinstance(viewer, TimeScatterView) + if not isinstance(viewer, PhaseScatterView): # ephemeris selected, but no active phase viewers return False - return viewer.reference.split('flux-vs-phase:')[1] == self.ephemeris_selected + return viewer._ephemeris_component == self.ephemeris_selected self.add_results.viewer.filters = [viewer_filter] + def _on_add_viewer(self, msg): + self._set_results_viewer() + self._live_update() + @observe('is_active', 'show_live_preview') def _toggle_marks(self, event={}): visible = self.show_live_preview and self.is_active - for viewer_id, mark in self.marks.items(): + for viewer_ref, mark in self.marks.items(): if not visible: this_visible = False elif self.ephemeris_selected == 'No ephemeris': this_visible = True else: - this_visible = viewer_id.split(':')[-1] == self.ephemeris_selected + viewer = self.app.get_viewer(viewer_ref) + viewer_ephem = getattr(viewer, '_ephemeris_component', None) + this_visible = viewer_ephem == self.ephemeris_selected mark.visible = this_visible @@ -260,10 +267,11 @@ def bin(self, add_data=True): if self.ephemeris_selected != 'No ephemeris': # prevent phase axis from becoming a time axis: - viewer_id = self.ephemeris_plugin._obj.phase_viewer_id - pv = self.app.get_viewer(viewer_id) + ephemeris_plugin = self.app._jdaviz_helper.plugins['Ephemeris'] phase_comp_lbl = self.app._jdaviz_helper._phase_comp_lbl(self.ephemeris_selected) - pv.state.x_att = self.app._jdaviz_helper._component_ids[phase_comp_lbl] + phase_comp = self.app._jdaviz_helper._component_ids[phase_comp_lbl] + for pv in ephemeris_plugin._obj._get_phase_viewers(self.ephemeris_selected): + pv.state.x_att = phase_comp # by resetting x_att, the preview marks may have dissappeared self._live_update() diff --git a/lcviz/plugins/ephemeris/ephemeris.py b/lcviz/plugins/ephemeris/ephemeris.py index b6b62f59..564277c5 100644 --- a/lcviz/plugins/ephemeris/ephemeris.py +++ b/lcviz/plugins/ephemeris/ephemeris.py @@ -130,7 +130,9 @@ def user_api(self): 'dataset', 'method', 'period_at_max_power', 'adopt_period_at_max_power'] return PluginUserApi(self, expose=expose) - def _phase_comp_lbl(self, component): + def _phase_comp_lbl(self, component=None): + if component is None: + component = self.component_selected if self.app._jdaviz_helper is None: # duplicate logic from helper in case this is ever called before the helper # is fully intialized @@ -139,20 +141,19 @@ def _phase_comp_lbl(self, component): @property def phase_comp_lbl(self): - return self._phase_comp_lbl(self.component_selected) + return self._phase_comp_lbl() - def _phase_viewer_id(self, component): - return f'flux-vs-phase:{component}' + def _generate_phase_viewer_id(self, component=None): + if component is None: + component = self.component_selected + return self.app._jdaviz_helper._get_clone_viewer_reference(f'flux-vs-phase:{component}') - @property - def phase_viewer_ids(self): - viewer_ids = self.app.get_viewer_ids() - return [self._phase_viewer_id(component) for component in self.component.choices - if self._phase_viewer_id(component) in viewer_ids] - - @property - def phase_viewer_id(self): - return self._phase_viewer_id(self.component_selected) + def _get_phase_viewers(self, lbl=None): + if lbl is None: + lbl = self.component_selected + return [viewer for vid, viewer in self.app._viewer_store.items() + if isinstance(viewer, PhaseScatterView) + and viewer._ephemeris_component == lbl] @property def default_phase_viewer(self): @@ -162,13 +163,6 @@ def default_phase_viewer(self): # ephemeris component return self._get_phase_viewers()[0] - def _get_phase_viewers(self, lbl=None): - if lbl is None: - lbl = self.component_selected - return [viewer for vid, viewer in self.app._viewer_store.items() - if isinstance(viewer, PhaseScatterView) - and viewer.ephemeris_component == lbl] - @property def ephemerides(self): return self._ephemerides @@ -269,50 +263,65 @@ def _update_all_phase_arrays(self, *args, ephem_component=None): dc.add_link(new_links) # update any plugin markers - # TODO: eventually might need to loop over multiple matching viewers - phase_viewer_id = self._phase_viewer_id(ephem_component) - if phase_viewer_id in self.app.get_viewer_ids(): - phase_viewer = self.app.get_viewer(phase_viewer_id) - for mark in phase_viewer.custom_marks: + for viewer in self._get_phase_viewers(ephem_component): + for mark in viewer.custom_marks: if hasattr(mark, 'update_phase_folding'): mark.update_phase_folding() return phase_comp_lbl - def create_phase_viewer(self): + def create_phase_viewer(self, ephem_component=None): """ Create a new phase viewer corresponding to ``component`` and populate the phase arrays with the current ephemeris, if necessary. + + Parameters + ---------- + ephem_component : str, optional + label of the component. If not provided or ``None``, will default to plugin value. """ - phase_viewer_id = self.phase_viewer_id + if ephem_component is None: + ephem_component = self.component_selected + phase_comp_lbl = self._phase_comp_lbl(ephem_component) dc = self.app.data_collection # check to see if this component already has a phase array. We'll just check the first # item in the data-collection since the rest of the logic in this plugin /should/ populate # the arrays across all entries. - if self.phase_comp_lbl not in [comp.label for comp in dc[0].components]: + if phase_comp_lbl not in [comp.label for comp in dc[0].components]: self.update_ephemeris() # calls _update_all_phase_arrays - create_phase_viewer = not self.phase_viewer_exists - if create_phase_viewer: - # TODO: stack horizontally by default? - self.app._on_new_viewer(NewViewerMessage(PhaseScatterView, data=None, sender=self.app), - vid=phase_viewer_id, name=phase_viewer_id) - - time_viewer_item = self.app._get_viewer_item(self.app._jdaviz_helper._default_time_viewer_reference_name) # noqa - for data in dc: - data_id = self.app._data_id_from_label(data.label) - visible = time_viewer_item['selected_data_items'].get(data_id, 'hidden') - self.app.set_data_visibility(phase_viewer_id, data.label, visible == 'visible') + phase_viewer_id = self._generate_phase_viewer_id(ephem_component) + # TODO: stack horizontally by default? + self.app._on_new_viewer(NewViewerMessage(PhaseScatterView, data=None, sender=self.app), + vid=phase_viewer_id, name=phase_viewer_id) + # access new viewer, set bookkeeping for ephemeris component pv = self.app.get_viewer(phase_viewer_id) - if create_phase_viewer: - pv.state.x_min, pv.state.x_max = (self.wrap_at-1, self.wrap_at) - pv.state.x_att = self.app._jdaviz_helper._component_ids[self.phase_comp_lbl] + pv._ephemeris_component = ephem_component + # since we couldn't set ephemeris_component right away, _check_if_phase_viewer_exists + # might be out-of-date + self._check_if_phase_viewer_exists() + + # set default data visibility + time_viewer_item = self.app._get_viewer_item(self.app._jdaviz_helper.default_time_viewer._obj.reference) # noqa + for data in dc: + data_id = self.app._data_id_from_label(data.label) + visible = time_viewer_item['selected_data_items'].get(data_id, 'hidden') + self.app.set_data_visibility(phase_viewer_id, data.label, visible == 'visible') + + # set x_att + phase_comp = self.app._jdaviz_helper._component_ids[phase_comp_lbl] + pv.state.x_att = phase_comp + + # set viewer limits + pv.state.x_min, pv.state.x_max = (self.wrap_at-1, self.wrap_at) + return pv.user_api def vue_create_phase_viewer(self, *args): - self.create_phase_viewer() + if not self.phase_viewer_exists: + self.create_phase_viewer() def vue_period_halve(self, *args): self.period /= 2 @@ -343,6 +352,7 @@ def _on_component_rename(self, old_lbl, new_lbl): viewer._ref_or_id.replace(old_lbl, new_lbl), update_id=True ) + viewer._ephemeris_component = new_lbl # update metadata entries so that they can be used for filtering applicable entries in # data menus @@ -410,7 +420,7 @@ def update_ephemeris(self, ephem_component=None, t0=None, period=None, dpdt=None Parameters ---------- - component : str, optional + ephem_component : str, optional label of the component. If not provided or ``None``, will default to plugin value. t0 : float, optional value of t0 to replace @@ -458,7 +468,8 @@ def round_to_1(x): return round(x, -int(np.floor(np.log10(abs(x))))) # if phase-viewer doesn't yet exist in the app, create it now - self.create_phase_viewer() + if not self.phase_viewer_exists: + self.create_phase_viewer() # update value in the dictionary (to support multi-ephems) if event: @@ -471,9 +482,10 @@ def round_to_1(x): if event.get('name') == 'wrap_at': old = event.get('old') if event.get('old') != '' else self._prev_wrap_at if event.get('new') != '': - pvs = self.default_phase_viewer.state delta_phase = event.get('new') - old - pvs.x_min, pvs.x_max = pvs.x_min + delta_phase, pvs.x_max + delta_phase + for pv in self._get_phase_viewers(): + pvs = pv.state + pvs.x_min, pvs.x_max = pvs.x_min + delta_phase, pvs.x_max + delta_phase # we need to cache the old value since it could become a string # if the widget is cleared self._prev_wrap_at = event.get('new') diff --git a/lcviz/plugins/flatten/flatten.py b/lcviz/plugins/flatten/flatten.py index 94f2d685..172ff904 100644 --- a/lcviz/plugins/flatten/flatten.py +++ b/lcviz/plugins/flatten/flatten.py @@ -102,26 +102,26 @@ def marks(self): trend_marks = {} flattened_marks = {} - for id, viewer in self.app._viewer_store.items(): + for viewer in self.app._viewer_store.values(): needs_trend = isinstance(viewer, TimeScatterView) and not isinstance(viewer, PhaseScatterView) # noqa needs_flattened = isinstance(viewer, (TimeScatterView, PhaseScatterView)) for mark in viewer.figure.marks: if isinstance(mark, LivePreviewTrend): - trend_marks[id] = mark + trend_marks[viewer.reference] = mark needs_trend = False elif isinstance(mark, LivePreviewFlattened): - flattened_marks[id] = mark + flattened_marks[viewer.reference] = mark needs_flattened = False if not needs_trend and not needs_flattened: break if needs_trend: mark = LivePreviewTrend(viewer, visible=self.is_active) viewer.figure.marks = viewer.figure.marks + [mark] - trend_marks[id] = mark + trend_marks[viewer.reference] = mark if needs_flattened: mark = LivePreviewFlattened(viewer, visible=self.is_active) viewer.figure.marks = viewer.figure.marks + [mark] - flattened_marks[id] = mark + flattened_marks[viewer.reference] = mark return trend_marks, flattened_marks diff --git a/lcviz/plugins/viewer_creator/__init__.py b/lcviz/plugins/viewer_creator/__init__.py new file mode 100644 index 00000000..b1c67ad2 --- /dev/null +++ b/lcviz/plugins/viewer_creator/__init__.py @@ -0,0 +1 @@ +from .viewer_creator import * # noqa diff --git a/lcviz/plugins/viewer_creator/viewer_creator.py b/lcviz/plugins/viewer_creator/viewer_creator.py new file mode 100644 index 00000000..d33a533e --- /dev/null +++ b/lcviz/plugins/viewer_creator/viewer_creator.py @@ -0,0 +1,48 @@ +from jdaviz.configs.default.plugins import ViewerCreator +from jdaviz.core.events import NewViewerMessage +from jdaviz.core.registries import tool_registry +from lcviz.events import EphemerisComponentChangedMessage +from lcviz.viewers import TimeScatterView + +__all__ = ['ViewerCreator'] + + +@tool_registry('lcviz-viewer-creator') +class ViewerCreator(ViewerCreator): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + self.hub.subscribe(self, EphemerisComponentChangedMessage, + handler=self._rebuild_available_viewers) + self._rebuild_available_viewers() + + def _rebuild_available_viewers(self, *args): + # filter to lcviz-specific viewers only + # list of dictionaries with name (registry name) + # and label (what appears in dropdown and the default label of the viewer) + + if self.app._jdaviz_helper is not None: + phase_viewers = [{'name': f'lcviz-phase-viewer:{e}', 'label': f'flux-vs-phase:{e}'} + for e in self.app._jdaviz_helper.plugins['Ephemeris'].component.choices] # noqa + else: + phase_viewers = [{'name': 'lcviz-phase-viewer:default', + 'label': 'flux-vs-phase:default'}] + + self.viewer_types = [v for v in self.viewer_types if v['name'].startswith('lcviz') + and not v['label'].startswith('flux-vs-phase')] + phase_viewers + self.send_state('viewer_types') + + def vue_create_viewer(self, name): + if name.startswith('lcviz-phase-viewer') or name.startswith('flux-vs-phase'): + ephem_comp = name.split(':')[1] + ephem_plg = self.app._jdaviz_helper.plugins['Ephemeris'] + ephem_plg.create_phase_viewer(ephem_comp) + return + if name in ('flux-vs-time', 'lcviz-time-viewer'): + # allow passing label and map to the name for upstream support + viewer_id = self.app._jdaviz_helper._get_clone_viewer_reference('flux-vs-time') + self.app._on_new_viewer(NewViewerMessage(TimeScatterView, data=None, sender=self.app), + vid=viewer_id, name=viewer_id) + return + + super().vue_create_viewer(name) diff --git a/lcviz/tests/test_parser.py b/lcviz/tests/test_parser.py index c4585e4f..e3c4cd20 100644 --- a/lcviz/tests/test_parser.py +++ b/lcviz/tests/test_parser.py @@ -70,7 +70,7 @@ def test_synthetic_lc(helper): def test_apply_xrangerois(helper, light_curve_like_kepler_quarter): lc = light_curve_like_kepler_quarter helper.load_data(lc) - viewer = helper.app.get_viewer(helper._default_time_viewer_reference_name) + viewer = helper.default_time_viewer._obj subset_plugin = helper.plugins['Subset Tools'] # the min/max of temporal regions can be defined in two ways: @@ -95,7 +95,7 @@ def test_apply_xrangerois(helper, light_curve_like_kepler_quarter): def test_apply_yrangerois(helper, light_curve_like_kepler_quarter): lc = light_curve_like_kepler_quarter helper.load_data(lc) - viewer = helper.app.get_viewer(helper._default_time_viewer_reference_name) + viewer = helper.default_time_viewer._obj subset_plugin = helper.plugins['Subset Tools'] subset_plugin._obj.subset_selected = "Create New" diff --git a/lcviz/tests/test_plugin_binning.py b/lcviz/tests/test_plugin_binning.py index 6011b7b7..7ce3bade 100644 --- a/lcviz/tests/test_plugin_binning.py +++ b/lcviz/tests/test_plugin_binning.py @@ -21,7 +21,7 @@ def test_docs_snippets(helper, light_curve_like_kepler_quarter): def test_plugin_binning(helper, light_curve_like_kepler_quarter): helper.load_data(light_curve_like_kepler_quarter) - tv = helper.app.get_viewer(helper._default_time_viewer_reference_name) + tv = helper.default_time_viewer._obj b = helper.plugins['Binning'] b._obj.plugin_opened = True @@ -32,7 +32,7 @@ def test_plugin_binning(helper, light_curve_like_kepler_quarter): with b.as_active(): assert b.ephemeris == 'No ephemeris' assert len(_get_marks_from_viewer(tv)) == 1 - assert len(_get_marks_from_viewer(pv)) == 0 + assert len(_get_marks_from_viewer(pv)) == 1 assert b._obj.ephemeris_dict == {} # update ephemeris will force re-phasing diff --git a/lcviz/tests/test_plugin_ephemeris.py b/lcviz/tests/test_plugin_ephemeris.py index 6fd7d69e..dea97b68 100644 --- a/lcviz/tests/test_plugin_ephemeris.py +++ b/lcviz/tests/test_plugin_ephemeris.py @@ -28,7 +28,7 @@ def test_plugin_ephemeris(helper, light_curve_like_kepler_quarter): ephem.period = 3.14 assert len(helper.app.get_viewer_ids()) == 2 assert ephem._obj.phase_viewer_exists - assert ephem._obj.phase_viewer_id in helper.app.get_viewer_ids() + assert 'flux-vs-phase:default' in helper.app.get_viewer_ids() ephem.t0 = 5 ephem._obj.vue_period_double() @@ -90,16 +90,50 @@ def test_cloned_phase_viewer(helper, light_curve_like_kepler_quarter): helper.load_data(light_curve_like_kepler_quarter) ephem = helper.plugins['Ephemeris'] + assert len(ephem._obj._get_phase_viewers()) == 0 pv1 = ephem.create_phase_viewer() + assert len(ephem._obj._get_phase_viewers()) == 1 pv2 = pv1._obj.clone_viewer() + assert len(ephem._obj._get_phase_viewers()) == 2 assert len(helper.viewers) == 3 assert pv1._obj.reference_id == 'flux-vs-phase:default' + assert pv1._obj._ephemeris_component == 'default' assert pv2._obj.reference_id == 'flux-vs-phase:default[1]' + assert pv2._obj._ephemeris_component == 'default' # renaming ephemeris should update both labels ephem.rename_component('default', 'renamed') assert pv1._obj.reference_id == 'flux-vs-phase:renamed' + assert pv1._obj._ephemeris_component == 'renamed' assert pv2._obj.reference_id == 'flux-vs-phase:renamed[1]' + assert pv2._obj._ephemeris_component == 'renamed' + assert len(ephem._obj._get_phase_viewers()) == 2 ephem.remove_component('renamed') assert len(helper.viewers) == 1 # just flux-vs-phase + + +def test_create_phase_viewer(helper, light_curve_like_kepler_quarter): + helper.load_data(light_curve_like_kepler_quarter) + ephem = helper.plugins['Ephemeris'] + vc = helper._tray_tools['lcviz-viewer-creator'] + + assert len(vc.viewer_types) == 2 # time viewer, phase viewer for default + _ = ephem.create_phase_viewer() + assert len(ephem._obj._get_phase_viewers()) == 1 + + vc.vue_create_viewer('flux-vs-phase:default') + assert len(ephem._obj._get_phase_viewers()) == 2 + for pv in ephem._obj._get_phase_viewers(): + assert pv._ephemeris_component == 'default' + + ephem.rename_component('default', 'renamed') + assert len(vc.viewer_types) == 2 + vc.vue_create_viewer('flux-vs-phase:renamed') + assert len(ephem._obj._get_phase_viewers()) == 3 + + for pv in ephem._obj._get_phase_viewers(): + assert pv._ephemeris_component == 'renamed' + + ephem.add_component('new') + assert len(vc.viewer_types) == 3 diff --git a/lcviz/tests/test_plugin_flatten.py b/lcviz/tests/test_plugin_flatten.py index 1be2e9a6..0d428b5d 100644 --- a/lcviz/tests/test_plugin_flatten.py +++ b/lcviz/tests/test_plugin_flatten.py @@ -25,7 +25,7 @@ def test_docs_snippets(helper, light_curve_like_kepler_quarter): def test_plugin_flatten(helper, light_curve_like_kepler_quarter): helper.load_data(light_curve_like_kepler_quarter) - tv = helper.app.get_viewer(helper._default_time_viewer_reference_name) + tv = helper.default_time_viewer._obj ephem = helper.plugins['Ephemeris'] pv = ephem.create_phase_viewer()._obj diff --git a/lcviz/tests/test_plugin_frequency_analysis.py b/lcviz/tests/test_plugin_frequency_analysis.py index 7fa9cec7..1d2766ac 100644 --- a/lcviz/tests/test_plugin_frequency_analysis.py +++ b/lcviz/tests/test_plugin_frequency_analysis.py @@ -19,7 +19,6 @@ def test_docs_snippets(helper, light_curve_like_kepler_quarter): def test_plugin_frequency_analysis(helper, light_curve_like_kepler_quarter): helper.load_data(light_curve_like_kepler_quarter) - # tv = helper.app.get_viewer(helper._default_time_viewer_reference_name) freq = helper.plugins['Frequency Analysis'] freq.open_in_tray() diff --git a/lcviz/tests/test_plugin_markers.py b/lcviz/tests/test_plugin_markers.py index 9ab8c071..b96fee1a 100644 --- a/lcviz/tests/test_plugin_markers.py +++ b/lcviz/tests/test_plugin_markers.py @@ -35,7 +35,7 @@ def test_docs_snippets(helper, light_curve_like_kepler_quarter): def test_plugin_markers(helper, light_curve_like_kepler_quarter): helper.load_data(light_curve_like_kepler_quarter) - tv = helper.app.get_viewer(helper._default_time_viewer_reference_name) + tv = helper.default_time_viewer._obj mp = helper.plugins['Markers'] label_mouseover = mp._obj.coords_info diff --git a/lcviz/tests/test_translator.py b/lcviz/tests/test_translator.py index cedf88b5..9ceb1f99 100644 --- a/lcviz/tests/test_translator.py +++ b/lcviz/tests/test_translator.py @@ -72,9 +72,7 @@ def test_round_trip(helper): '2009-05-02 03:52' ], format='iso') - viewer = helper.app.get_viewer( - helper._default_time_viewer_reference_name - ) + viewer = helper.default_time_viewer._obj viewer.apply_roi(XRangeROI(*near_transit)) columns_to_check = ['time', 'flux', 'flux_err'] diff --git a/lcviz/tests/test_tray_viewer_creator.py b/lcviz/tests/test_tray_viewer_creator.py new file mode 100644 index 00000000..e2a6b835 --- /dev/null +++ b/lcviz/tests/test_tray_viewer_creator.py @@ -0,0 +1,9 @@ +def test_tray_viewer_creator(helper, light_curve_like_kepler_quarter): + # additional coverage in test_plugin_ephemeris + helper.load_data(light_curve_like_kepler_quarter) + vc = helper._tray_tools['lcviz-viewer-creator'] + + assert len(helper.viewers) == 1 + assert len(vc.viewer_types) == 2 # time and default phase + vc.vue_create_viewer('flux-vs-time') + assert len(helper.viewers) == 2 diff --git a/lcviz/tests/test_viewers.py b/lcviz/tests/test_viewers.py index 71d3fca3..e162ebdd 100644 --- a/lcviz/tests/test_viewers.py +++ b/lcviz/tests/test_viewers.py @@ -1,7 +1,7 @@ def test_reset_limits(helper, light_curve_like_kepler_quarter): helper.load_data(light_curve_like_kepler_quarter) - tv = helper.app.get_viewer(helper._default_time_viewer_reference_name) + tv = helper.default_time_viewer._obj orig_xlims = (tv.state.x_min, tv.state.x_max) orig_ylims = (tv.state.y_min, tv.state.y_max) @@ -23,7 +23,7 @@ def test_clone(helper, light_curve_like_kepler_quarter): helper.load_data(light_curve_like_kepler_quarter) def_viewer = helper.viewers['flux-vs-time'] - assert def_viewer._obj._get_clone_viewer_reference() == 'flux-vs-time[1]' + assert helper._get_clone_viewer_reference(def_viewer._obj.reference) == 'flux-vs-time[1]' new_viewer = def_viewer._obj.clone_viewer() - assert new_viewer._obj._get_clone_viewer_reference() == 'flux-vs-time[2]' + assert helper._get_clone_viewer_reference(new_viewer._obj.reference) == 'flux-vs-time[2]' diff --git a/lcviz/viewers.py b/lcviz/viewers.py index ee70f3b6..fa853c74 100644 --- a/lcviz/viewers.py +++ b/lcviz/viewers.py @@ -210,17 +210,8 @@ def apply_roi(self, roi, use_current=False): super().apply_roi(roi, use_current=use_current) - def _get_clone_viewer_reference(self): - base_name = self.reference.split("[")[0] - name = base_name - ind = 0 - while name in self.jdaviz_helper.viewers.keys(): - ind += 1 - name = f"{base_name}[{ind}]" - return name - def clone_viewer(self): - name = self._get_clone_viewer_reference() + name = self.jdaviz_helper._get_clone_viewer_reference(self.reference) self.jdaviz_app._on_new_viewer(NewViewerMessage(self.__class__, data=None, @@ -236,7 +227,9 @@ def clone_viewer(self): # TODO: don't revert color when adding same data to a new viewer # (same happens when creating a phase-viewer from ephemeris plugin) - new_viewer = self.jdaviz_helper.viewers[name]._obj + new_viewer = self.jdaviz_app.get_viewer(name) + if hasattr(self, 'ephemeris_component'): + new_viewer._ephemeris_component = self._ephemeris_component for k, v in this_state.items(): if k in ('layers',): continue @@ -245,15 +238,15 @@ def clone_viewer(self): return new_viewer.user_api -@viewer_registry("lcviz-phase-viewer", label="phase-vs-time") +@viewer_registry("lcviz-phase-viewer", label="flux-vs-phase") class PhaseScatterView(TimeScatterView): - @property - def ephemeris_component(self): - return self.reference.split('[')[0].split(':')[-1] + def __init__(self, *args, **kwargs): + self._ephemeris_component = 'default' + super().__init__(*args, **kwargs) def _set_plot_x_axes(self, dc, component_labels, light_curve): # setting of y_att will be handled by ephemeris plugin - self.state.x_att = dc[0].components[component_labels.index(f'phase:{self.ephemeris_component}')] # noqa + self.state.x_att = dc[0].components[component_labels.index(f'phase:{self._ephemeris_component}')] # noqa self.figure.axes[0].label = 'phase' self.figure.axes[0].num_ticks = 5 @@ -262,4 +255,4 @@ def times_to_phases(self, times): if ephem is None: raise ValueError("must have ephemeris plugin loaded to convert") - return ephem.times_to_phases(times, ephem_component=self.ephemeris_component) + return ephem.times_to_phases(times, ephem_component=self._ephemeris_component)