Skip to content

Commit

Permalink
code cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
kecnry committed Jul 12, 2023
1 parent baa1eaa commit b5f52ad
Show file tree
Hide file tree
Showing 23 changed files with 131 additions and 131 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ Bug Fixes

- Fixed ``cls`` input being ignored in ``viz.get_data()``. [#2242]

- Live-previews and keypress events that depend on the plugin being opened now work for inline
and popout windows. [#2295]

Cubeviz
^^^^^^^

Expand Down
2 changes: 1 addition & 1 deletion docs/dev/ui_style_guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Tray Plugins
In order to be consistent with layout, styling, and spacing, UI development on plugins should
try to adhere to the following principles:

* Any tray plugin should utilize ``<j-tray-plugin :has_previews="has_previews" :plugin_ping.sync="plugin_ping" :persistent_previews.sync="persistent_previews" :disabled_msg='disabled_msg' :popout_button="popout_button">`` as the
* Any tray plugin should utilize ``<j-tray-plugin :uses_active_status="uses_active_status" :plugin_ping.sync="plugin_ping" :keep_active.sync="keep_active" :disabled_msg='disabled_msg' :popout_button="popout_button">`` as the
outer-container (which provides consistent styling rules). Any changes to style
across all plugins should then take place in the
``j-tray-plugin`` stylesheet (``jdaviz/components/tray_plugin.vue``).
Expand Down
14 changes: 7 additions & 7 deletions jdaviz/components/tray_plugin.vue
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
<span> {{ getDisabledMsg() }}</span>
</v-row>
<div v-else>
<v-row v-if="has_previews">
<v-row v-if="uses_active_status">
<v-switch
v-model="persistent_previews"
@change="$emit('update:persistent_previews', $event)"
label="persistent live-preview"
hint="show live-preview even when plugin is not opened"
v-model="keep_active"
@change="$emit('update:keep_active', $event)"
label="keep active"
hint="consider plugin active (showing any previews and enabling all keypress events) even when not opened"
persistent-hint>
</v-switch>
</v-row>
Expand All @@ -30,7 +30,7 @@
<script>
module.exports = {
props: ['disabled_msg', 'description', 'link', 'popout_button',
'has_previews', 'plugin_ping', 'persistent_previews'],
'uses_active_status', 'plugin_ping', 'keep_active'],
methods: {
isDisabled() {
return this.getDisabledMsg().length > 0
Expand All @@ -45,7 +45,7 @@ module.exports = {
this.$emit('update:plugin_ping', Date.now())
setTimeout(() => {
this.sendPing()
}, 100) // ms
}, 200) // ms
}
},
mounted() {
Expand Down
4 changes: 2 additions & 2 deletions jdaviz/configs/default/plugins/line_lists/line_lists.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,9 @@ def _on_rs_redshift_updated(self, event):
msg = RedshiftMessage("redshift", value, sender=self)
self.app.hub.broadcast(msg)

@observe('plugin_active')
@observe('plugin_opened')
def _update_line_list_obs(self, *args):
if not self.plugin_active:
if not self.plugin_opened:
return

for list_name, line_list in self.list_contents.items():
Expand Down
12 changes: 6 additions & 6 deletions jdaviz/configs/default/plugins/markers/markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class Markers(PluginTemplateMixin, ViewerSelectMixin, TableMixin):
* :meth:`~jdaviz.core.template_mixin.TableMixin.export_table`
"""
template_file = __file__, "markers.vue"
has_previews = Bool(True).tag(sync=True)
uses_active_status = Bool(True).tag(sync=True)

_default_table_values = {'spectral_axis': np.nan,
'spectral_axis:unit': '',
Expand Down Expand Up @@ -80,7 +80,7 @@ def __init__(self, *args, **kwargs):
self.hub.subscribe(self, ViewerAddedMessage, handler=self._on_viewer_added)

def _create_viewer_callbacks(self, viewer):
if not self.show_previews:
if not self.is_active:
return

callback = self._viewer_callback(viewer, self._on_viewer_key_event)
Expand All @@ -107,14 +107,14 @@ def marks(self):
def coords_info(self):
return self.app.session.application._tools['g-coords-info']

@observe('show_previews')
def _on_show_previews_changed(self, *args):
@observe('is_active')
def _on_is_active_changed(self, *args):
if self.disabled_msg:
return

# toggle visibility of markers
for mark in self.marks.values():
mark.visible = self.show_previews
mark.visible = self.is_active

# subscribe/unsubscribe to keypress events across all viewers
for viewer in self.app._viewer_store.values():
Expand All @@ -123,7 +123,7 @@ def _on_show_previews_changed(self, *args):
continue
callback = self._viewer_callback(viewer, self._on_viewer_key_event)

if self.show_previews:
if self.is_active:
viewer.add_event_callback(callback, events=['keydown'])
else:
viewer.remove_event_callback(callback)
Expand Down
4 changes: 2 additions & 2 deletions jdaviz/configs/default/plugins/markers/markers.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
<j-tray-plugin
description='Create and export markers. Press "m" with the cursor over a viewer to log the mouseover information. To change the selected layer, click the layer cycler in the mouseover information section of the app-level toolbar.'
:link="'https://jdaviz.readthedocs.io/en/'+vdocs+'/'+config+'/plugins.html#markers'"
:has_previews="has_previews"
:uses_active_status="uses_active_status"
:plugin_ping.sync="plugin_ping"
:persistent_previews.sync="persistent_previews"
:keep_active.sync="keep_active"
:popout_button="popout_button">

<jupyter-widget :widget="table_widget"></jupyter-widget>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def test_markers_cubeviz(cubeviz_helper, spectrum1d_cube):
label_mouseover = cubeviz_helper.app.session.application._tools['g-coords-info']

mp = cubeviz_helper.plugins['Markers']
mp.open_in_tray()
mp.keep_active = True

# test event in flux viewer
label_mouseover._viewer_mouse_event(fv,
Expand Down Expand Up @@ -112,8 +112,8 @@ def test_markers_cubeviz(cubeviz_helper, spectrum1d_cube):
assert len(_get_markers_from_viewer(fv).x) == 1
assert len(_get_markers_from_viewer(sv).x) == 2

# markers hide when plugin closed (since persistent_previews == False)
cubeviz_helper.app.state.drawer = False
# markers hide when plugin closed and keep_active = False
mp.keep_active = False
assert _get_markers_from_viewer(fv).visible is False
assert _get_markers_from_viewer(sv).visible is False

Expand Down
14 changes: 6 additions & 8 deletions jdaviz/configs/default/plugins/plot_options/plot_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,10 +445,6 @@ def user_api(self):

return PluginUserApi(self, expose)

def show(self, *args, **kwargs):
super().show(*args, **kwargs)
self._update_stretch_histogram()

@observe('show_viewer_labels')
def _on_show_viewer_labels_changed(self, event):
self.app.state.settings['viewer_labels'] = event['new']
Expand Down Expand Up @@ -483,15 +479,18 @@ def vue_set_value(self, data):
value = data.get('value')
setattr(self, attr_name, value)

@observe('plugin_opened_in_tray', 'layer_selected', 'viewer_selected', 'stretch_hist_zoom_limits')
@observe('plugin_opened', 'layer_selected', 'viewer_selected',
'stretch_hist_zoom_limits')
def _update_stretch_histogram(self, msg={}):
if not self.stretch_function_sync.get('in_subscribed_states'): # pragma: no cover
# no (image) viewer with stretch function options
return
if not hasattr(self, 'viewer'): # pragma: no cover
# plugin hasn't been fully initialized yet
return
if (not self.viewer.selected or not self.layer.selected): # pragma: no cover
if (not self.plugin_opened
or not self.viewer.selected
or not self.layer.selected): # pragma: no cover
# no need to make updates, updates will be redrawn when plugin is opened
# NOTE: this won't update when the plugin is shown but not open in the tray
return
Expand All @@ -505,8 +504,7 @@ def _update_stretch_histogram(self, msg={}):
or len(self.layer.selected) > 1): # pragma: no cover
# currently only support single-layer/viewer. For now we'll just clear and return.
# TODO: add support for multi-layer/viewer
if self.stretch_histogram is not None:
bqplot_clear_figure(self.stretch_histogram)
bqplot_clear_figure(self.stretch_histogram)
return

viewer = self.viewer.selected_obj[0] if self.multiselect else self.viewer.selected_obj
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def test_multiselect(cubeviz_helper, spectrum1d_cube):
def test_stretch_histogram(cubeviz_helper, spectrum1d_cube_with_uncerts):
cubeviz_helper.load_data(spectrum1d_cube_with_uncerts)
po = cubeviz_helper.app.get_tray_item_from_name('g-plot-options')
po.open_in_tray()

assert po.stretch_histogram is not None

Expand Down
8 changes: 2 additions & 6 deletions jdaviz/configs/imviz/plugins/compass/compass.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ def __init__(self, *args, **kwargs):
def user_api(self):
return PluginUserApi(self, expose=('viewer',), readonly=('data_label',))

def show(self, *args, **kwargs):
super().show(*args, **kwargs)
self._compass_with_new_viewer(from_show=True)

def _on_viewer_data_changed(self, msg=None):
if self.viewer_selected:
viewer = self.viewer.selected_obj
Expand All @@ -61,15 +57,15 @@ def _set_compass_rotation(self):
self.canvas_angle = viewer_item.get('canvas_angle', 0) # noqa
self.canvas_flip_horizontal = viewer_item.get('canvas_flip_horizontal', False)

@observe("viewer_selected", "plugin_opened_in_tray")
@observe("viewer_selected", "plugin_opened")
def _compass_with_new_viewer(self, *args, **kwargs):
if not hasattr(self, 'viewer'):
# mixin object not yet initialized
return

# There can be only one!
for vid, viewer in self.app._viewer_store.items():
if vid == self.viewer.selected_id and (self.plugin_opened_in_tray or kwargs.get('from_show')): # noqa
if vid == self.viewer.selected_id and self.plugin_opened:
viewer.compass = self
viewer.on_limits_change() # Force redraw

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ def reset_results(self):
self.plot_across_y.clear_all_marks()

# This is also triggered from viewer code.
@observe("selected_viewer")
@observe("plugin_opened", "selected_viewer")
def vue_draw_plot(self, *args, **kwargs):
"""Draw line profile plots for given Data across given X and Y indices (0-indexed)."""
if not self.selected_x or not self.selected_y:
if not self.selected_x or not self.selected_y or not self.plugin_opened:
return

viewer = self.app.get_viewer_by_id(self.selected_viewer)
Expand Down
2 changes: 1 addition & 1 deletion jdaviz/configs/imviz/plugins/viewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def on_mouse_or_key_event(self, data):
self.blink_once(reversed=key_pressed=='B') # noqa: E225

# TODO: move into plugin callback?
elif key_pressed == 'l':
elif key_pressed == 'l' and self.line_profile_xy.plugin_opened:
# Same data as mousemove above.
image = self.active_image_layer.layer
x = data['domain']['x']
Expand Down
6 changes: 3 additions & 3 deletions jdaviz/configs/imviz/tests/test_catalogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def test_plugin_no_image(self, imviz_helper):
self.imviz = imviz_helper

catalogs_plugin = self.imviz.app.get_tray_item_from_name('imviz-catalogs')
catalogs_plugin.plugin_active = True
catalogs_plugin.open_in_tray()
# running the search without any data loaded into Imviz
catalogs_plugin.vue_do_search()

Expand All @@ -54,7 +54,7 @@ def test_plugin_image_no_result(self, imviz_helper, image_2d_wcs):
self.imviz = imviz_helper

catalogs_plugin = self.imviz.app.get_tray_item_from_name('imviz-catalogs')
catalogs_plugin.plugin_active = True
catalogs_plugin.open_in_tray()
catalogs_plugin.vue_do_search()

# number of results should be 0
Expand Down Expand Up @@ -91,7 +91,7 @@ def test_plugin_image_with_result(self, imviz_helper, tmp_path):
self.imviz = imviz_helper

catalogs_plugin = self.imviz.app.get_tray_item_from_name('imviz-catalogs')
catalogs_plugin.plugin_active = True
catalogs_plugin.open_in_tray()
# This basically calls the following under the hood:
# skycoord_center = SkyCoord(6.62754354, 1.54466139, unit="deg")
# zoom_radius = r_max = 3 * u.arcmin
Expand Down
4 changes: 2 additions & 2 deletions jdaviz/configs/imviz/tests/test_line_profile_xy.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class TestLineProfileXY(BaseImviz_WCS_NoWCS):
def test_plugin_linked_by_pixel(self):
"""Go through plugin logic but does not check plot contents."""
lp_plugin = self.imviz.app.get_tray_item_from_name('imviz-line-profile-xy')
lp_plugin.plugin_active = True
lp_plugin.open_in_tray()

lp_plugin._on_viewers_changed() # Populate plugin menu items.
assert lp_plugin.viewer_items == ['imviz-0']
Expand Down Expand Up @@ -66,7 +66,7 @@ def test_plugin_linked_by_pixel(self):
assert lp_plugin.plot_available

# Nothing should update on "l" when plugin closed.
lp_plugin.plugin_active = False
lp_plugin.plugin_opened = False
self.viewer.on_mouse_or_key_event(
{'event': 'keydown', 'key': 'l', 'domain': {'x': 5.1, 'y': 5}})
lp_plugin.selected_x = '1.1'
Expand Down
24 changes: 12 additions & 12 deletions jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class LineAnalysis(PluginTemplateMixin, DatasetSelectMixin, SpectralSubsetSelect
"""
dialog = Bool(False).tag(sync=True)
template_file = __file__, "line_analysis.vue"
has_previews = Bool(True).tag(sync=True)
uses_active_status = Bool(True).tag(sync=True)

spatial_subset_items = List().tag(sync=True)
spatial_subset_selected = Unicode().tag(sync=True)
Expand Down Expand Up @@ -206,26 +206,26 @@ def _on_viewer_subsets_changed(self, msg):
if (msg.subset.label in [self.spectral_subset_selected,
self.spatial_subset_selected,
self.continuum_subset_selected]
and (self.show_previews or self.plugin_opened_in_tray)):
and self.is_active):
self._calculate_statistics()

def _on_global_display_unit_changed(self, msg):
if self.show_previews:
if self.is_active:
self._calculate_statistics()

@observe('show_previews')
def _show_previews_changed(self, *args):
@observe('is_active')
def _is_active_changed(self, msg):
if self.disabled_msg:
return

for pos, mark in self.marks.items():
mark.visible = self.show_previews
if self.show_previews:
mark.visible = self.is_active
if self.is_active:
self._calculate_statistics()

@deprecated(since="3.6", alternative="persistent_previews")
@deprecated(since="3.6", alternative="keep_active")
def show_continuum_marks(self):
self.persistent_previews = True
self.keep_active = True

@property
def marks(self):
Expand All @@ -247,9 +247,9 @@ def marks(self):
return {}
# then haven't been initialized yet, so initialize with empty
# marks that will be populated once the first analysis is done.
marks = {'left': LineAnalysisContinuumLeft(viewer, visible=self.show_previews),
'center': LineAnalysisContinuumCenter(viewer, visible=self.show_previews),
'right': LineAnalysisContinuumRight(viewer, visible=self.show_previews)}
marks = {'left': LineAnalysisContinuumLeft(viewer, visible=self.is_active),
'center': LineAnalysisContinuumCenter(viewer, visible=self.is_active),
'right': LineAnalysisContinuumRight(viewer, visible=self.is_active)}
shadows = [ShadowLine(mark, shadow_width=2) for mark in marks.values()]
# NOTE: += won't trigger the figure to notice new marks
viewer.figure.marks = viewer.figure.marks + shadows + list(marks.values())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
<j-tray-plugin
description="Return statistics for a single spectral line."
:link="'https://jdaviz.readthedocs.io/en/'+vdocs+'/'+config+'/plugins.html#line-analysis'"
:has_previews="has_previews"
:uses_active_status="uses_active_status"
:plugin_ping.sync="plugin_ping"
:persistent_previews.sync="persistent_previews"
:keep_active.sync="keep_active"
:disabled_msg="disabled_msg"
:popout_button="popout_button">

Expand Down
Loading

0 comments on commit b5f52ad

Please sign in to comment.