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

Generalize units in slice plugin #2706

Merged
merged 13 commits into from
Feb 16, 2024
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ Cubeviz
- ``spatial_subset`` in the spectral extraction plugin is now renamed to ``aperture`` and the deprecated name will
be removed in a future release. [#2664]

- Slice plugin's ``wavelength``, ``wavelength_unit``, and ``show_wavelength`` are deprecated in favor
of ``value``, ``value_unit``, and ``show_value``, respectively. [#2706]

Imviz
^^^^^

Expand Down
10 changes: 2 additions & 8 deletions jdaviz/configs/cubeviz/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,8 @@ def select_wavelength(self, wavelength):
"""
if not isinstance(wavelength, (int, float)):
raise TypeError("wavelength must be a float or int")
# Retrieve the x slices from the spectrum viewer's marks
sv = self.app.get_viewer(self._default_spectrum_viewer_reference_name)
x_all = sv.native_marks[0].x
if sv.state.layers[0].as_steps:
# then the marks have been doubled in length (each point duplicated)
x_all = x_all[::2]
index = np.argmin(abs(x_all - wavelength))
return self.select_slice(int(index))
msg = SliceSelectSliceMessage(value=wavelength, sender=self)
self.app.hub.broadcast(msg)

@property
def specviz(self):
Expand Down
112 changes: 67 additions & 45 deletions jdaviz/configs/cubeviz/plugins/slice/slice.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
import numpy as np
import astropy.units as u
from astropy.units import UnitsWarning
from astropy.utils.decorators import deprecated
from glue_jupyter.bqplot.image import BqplotImageView
from glue_jupyter.bqplot.profile import BqplotProfileView
from traitlets import Bool, observe, Any, Int
from traitlets import Any, Bool, Int, Unicode, observe
from specutils.spectra.spectrum1d import Spectrum1D

from jdaviz.core.events import (AddDataMessage, SliceToolStateMessage,
SliceSelectSliceMessage, SliceWavelengthUpdatedMessage,
SliceSelectSliceMessage, SliceValueUpdatedMessage,
GlobalDisplayUnitChanged)
from jdaviz.core.registries import tray_registry
from jdaviz.core.template_mixin import PluginTemplateMixin
Expand All @@ -33,24 +34,26 @@
* :meth:`~jdaviz.core.template_mixin.PluginTemplateMixin.close_in_tray`
* ``slice``
Current slice number.
* ``wavelength``
Wavelength of the current slice. When setting this directly, it will update automatically to
the wavelength corresponding to the nearest slice.
* ``value``
Value (wavelength or frequency) of the current slice. When setting this directly, it will
update automatically to the value corresponding to the nearest slice.
* ``show_indicator``
Whether to show indicator in spectral viewer when slice tool is inactive.
* ``show_wavelength``
Whether to show slice wavelength in label to right of indicator.
* ``show_value``
Whether to show slice value in label to right of indicator.
"""
template_file = __file__, "slice.vue"
slice = Any(0).tag(sync=True)
wavelength = Any(-1).tag(sync=True)
wavelength_unit = Any("").tag(sync=True)
min_value = Int(0).tag(sync=True)
max_value = Int(100).tag(sync=True)
value = Any(-1).tag(sync=True)
value_label = Unicode("Wavelength").tag(sync=True)
value_unit = Any("").tag(sync=True)

min_slice = Int(0).tag(sync=True)
max_slice = Int(100).tag(sync=True)
wait = Int(200).tag(sync=True)

show_indicator = Bool(True).tag(sync=True)
show_wavelength = Bool(True).tag(sync=True)
show_value = Bool(True).tag(sync=True)

is_playing = Bool(False).tag(sync=True)
play_interval = Int(200).tag(sync=True) # milliseconds
Expand Down Expand Up @@ -81,22 +84,30 @@
self.session.hub.subscribe(self, AddDataMessage,
handler=self._on_data_added)

# update internal wavelength when x display unit is changed (preserving slice)
# update internal value (wavelength/frequency) when x display unit is changed
# so that the current slice number is preserved
self.session.hub.subscribe(self, GlobalDisplayUnitChanged,
handler=self._on_global_display_unit_changed)

@property
def _default_spectrum_viewer_reference_name(self):
return self.app._jdaviz_helper_default_spectrum_viewer_reference_name
@deprecated(since="3.9", alternative="value")
def wavelength(self):
return self.user_api.value

Check warning on line 95 in jdaviz/configs/cubeviz/plugins/slice/slice.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/cubeviz/plugins/slice/slice.py#L95

Added line #L95 was not covered by tests

@property
@deprecated(since="3.9", alternative="value_unit")
def wavelength_unit(self):
return self.user_api.value_unit

Check warning on line 100 in jdaviz/configs/cubeviz/plugins/slice/slice.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/cubeviz/plugins/slice/slice.py#L100

Added line #L100 was not covered by tests

@property
def _default_image_viewer_reference_name(self):
return self.app._jdaviz_helper._default_image_viewer_reference_name
@deprecated(since="3.9", alternative="show_value")
def show_wavelength(self):
return self.user_api.show_value

Check warning on line 105 in jdaviz/configs/cubeviz/plugins/slice/slice.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/cubeviz/plugins/slice/slice.py#L105

Added line #L105 was not covered by tests

@property
def user_api(self):
return PluginUserApi(self, expose=('slice', 'wavelength',
'show_indicator', 'show_wavelength'))
return PluginUserApi(self, expose=('slice', 'wavelength', 'value',
'show_indicator', 'show_wavelength', 'show_value'))

@property
def slice_indicator(self):
Expand All @@ -114,7 +125,7 @@
self._watched_viewers.remove(viewer)
elif isinstance(viewer, BqplotProfileView) and watch:
if self._x_all is None and len(viewer.data()):
# cache wavelengths so that wavelength <> slice conversion can be done efficiently
# cache values (wavelengths/freqs) so that value <> slice conversion is efficient
self._update_data(viewer.data()[0].spectral_axis)

if viewer not in self._indicator_viewers:
Expand All @@ -126,9 +137,9 @@
def _on_data_added(self, msg):
if isinstance(msg.viewer, BqplotImageView):
if len(msg.data.shape) == 3:
self.max_value = msg.data.shape[-1] - 1 # Same as i_end in Export Plot plugin
self.max_slice = msg.data.shape[-1] - 1 # Same as i_end in Export Plot plugin
self._watch_viewer(msg.viewer, True)
msg.viewer.state.slices = (0, 0, int(self.slice))
self._set_viewer_to_slice(msg.viewer, int(self.slice))

elif isinstance(msg.viewer, BqplotProfileView):
self._watch_viewer(msg.viewer, True)
Expand All @@ -141,18 +152,18 @@
def _update_data(self, x_all):
self._x_all = x_all.value

if self.wavelength == -1:
if self.value == -1:
if len(x_all):
# initialize at middle of cube
self.slice = int(len(x_all)/2)
else:
# leave in the pre-init state and don't update the wavelength/slice
# leave in the pre-init state and don't update the value/slice
return

# Also update unit when data is updated
self.wavelength_unit = x_all.unit.to_string()
self.value_unit = x_all.unit.to_string()

# force wavelength to update from the current slider value
# force value (wavelength/frequency) to update from the current slider slice
self._on_slider_updated({'new': self.slice})

# update data held inside slice indicator and force reverting to original active status
Expand All @@ -168,70 +179,81 @@

def _on_select_slice_message(self, msg):
# NOTE: by setting the slice index, the observer (_on_slider_updated)
# will sync across all viewers and update the wavelength
# will sync across all viewers and update the value (wavelength/frequency)
with warnings.catch_warnings():
warnings.simplefilter('ignore', category=UnitsWarning)
self.slice = msg.slice
if msg.slice is not None:
self.slice = msg.slice
elif msg.value is not None:
self.value = msg.value

@property
def slice_axis(self):
return 'spectral'

Check warning on line 192 in jdaviz/configs/cubeviz/plugins/slice/slice.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/cubeviz/plugins/slice/slice.py#L192

Added line #L192 was not covered by tests

def _on_global_display_unit_changed(self, msg):
if msg.axis != 'spectral':
if msg.axis != self.slice_axis:

Check warning on line 195 in jdaviz/configs/cubeviz/plugins/slice/slice.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/cubeviz/plugins/slice/slice.py#L195

Added line #L195 was not covered by tests
return
prev_unit = self.wavelength_unit
prev_unit = self.value_unit

Check warning on line 197 in jdaviz/configs/cubeviz/plugins/slice/slice.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/cubeviz/plugins/slice/slice.py#L197

Added line #L197 was not covered by tests
# original unit during init can be blank or deg (before axis is set correctly)
if self._x_all is None or prev_unit in ('deg', ''):
return
self._update_data((self._x_all * u.Unit(prev_unit)).to(msg.unit, u.spectral()))

@observe('wavelength')
def _on_wavelength_updated(self, event):
@observe('value')
def _on_value_updated(self, event):
# convert to float (JS handles stripping any invalid characters)
try:
value = float(event.get('new'))
except ValueError:
# do not accept changes, we'll revert via the slider
# since this @change event doesn't have access to
# the old value, and self.wavelength already updated
# the old value, and self.value already updated
# via the v-model
self._on_slider_updated({'new': self.slice})
return

# NOTE: by setting the index, this should recursively update the
# wavelength to the nearest applicable value in _on_slider_updated
# value (wavelength/frequency) to the nearest applicable value in _on_slider_updated
self.slice = int(np.argmin(abs(value - self._x_all)))

@observe('show_indicator', 'show_wavelength')
@observe('show_indicator', 'show_value')
def _on_setting_changed(self, event):
msg = SliceToolStateMessage({event['name']: event['new']}, sender=self)
self.session.hub.broadcast(msg)

def _set_viewer_to_slice(self, viewer, value):
viewer.state.slices = (0, 0, value)

@observe('slice')
def _on_slider_updated(self, event):
if self._x_all is None:
return

value = int(event.get('new', int(len(self._x_all)/2))) % (int(self.max_value) + 1)
value = int(event.get('new', int(len(self._x_all)/2))) % (int(self.max_slice) + 1)

self.wavelength = self._x_all[value]
self.value = self._x_all[value]

for viewer in self._watched_viewers:
viewer.state.slices = (0, 0, value)
self._set_viewer_to_slice(viewer, value)
for viewer in self._indicator_viewers:
viewer._update_slice_indicator(value)
if hasattr(viewer, 'slice_indicator'):
viewer.slice_indicator.slice = value

self.hub.broadcast(SliceWavelengthUpdatedMessage(slice=value,
wavelength=self.wavelength,
wavelength_unit=self.wavelength_unit,
sender=self))
self.hub.broadcast(SliceValueUpdatedMessage(slice=value,
value=self.value,
value_unit=self.value_unit,
sender=self))

def vue_goto_first(self, *args):
if self.is_playing:
return
self._on_slider_updated({'new': self.min_value})
self._on_slider_updated({'new': self.min_slice})

def vue_goto_last(self, *args):
if self.is_playing:
return
self._on_slider_updated({'new': self.max_value})
self._on_slider_updated({'new': self.max_slice})

def vue_play_next(self, *args):
if self.is_playing:
Expand Down
56 changes: 24 additions & 32 deletions jdaviz/configs/cubeviz/plugins/slice/slice.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<j-tray-plugin
description='Select slice (or wavelength) of the cube to show in the image viewers and highlighted in the spectrum viewer. The slice can also be changed interactively in the spectrum viewer by activating the slice tool.'
description='Select slice of the cube to show in the image viewers. The slice can also be changed interactively in the spectrum viewer by activating the slice tool.'
:link="docs_link || 'https://jdaviz.readthedocs.io/en/'+vdocs+'/'+config+'/plugins.html#slice'"
:popout_button="popout_button">

Expand All @@ -14,16 +14,16 @@
<v-row>
<v-switch
label="Show Indicator"
hint="Show indicator in spectral viewer even when slice tool is inactive."
hint="Show slice indicator even when slice tool is inactive."
v-model="show_indicator"
persistent-hint>
</v-switch>
</v-row>
<v-row>
<v-switch
label="Show Wavelength"
hint="Show slice wavelength in label to right of indicator."
v-model="show_wavelength"
label="Show Value"
:hint="'Show slice '+value_label.toLowerCase()+' in label to right of indicator.'"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the need for toLowerCase here.

Copy link
Member Author

@kecnry kecnry Feb 15, 2024

Choose a reason for hiding this comment

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

Because we want lower case here (not "Show slice Wavelength in label..."), but we want title case in the actual input widget, without defining two different strings on the python-side.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, just to make sure, this does not affect string representation of the unit, right? Because "MJy" is not the same as "mjy".

Copy link
Member Author

@kecnry kecnry Feb 15, 2024

Choose a reason for hiding this comment

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

right, this is the label (Wavelength, Frequency, Time, etc), not the unit.

v-model="show_value"
persistent-hint>
</v-switch>
</v-row>
Expand All @@ -37,39 +37,31 @@
:value="slice"
@input="throttledSetValue"
class="align-center"
:max="max_value"
:min="min_value"
:max="max_slice"
:min="min_slice"
hide-details
/>
</v-row>

<v-row class="row-no-outside-padding row-min-bottom-padding">
<v-col>
<v-text-field
v-model.number="slice"
class="mt-0 pt-0"
type="number"
label="Slice"
hint="Slice number"
></v-text-field>
</v-col>
<v-col cols=3>
<span> / {{ max_value }}</span>
</v-col>
<v-row>
<v-text-field
v-model.number="slice"
class="mt-0 pt-0"
type="number"
label="Slice"
hint="Slice number"
:suffix="'/'+max_slice"
></v-text-field>
</v-row>

<v-row class="row-no-outside-padding">
<v-col>
<v-text-field
v-model="wavelength"
class="mt-0 pt-0"
label="Wavelength"
hint="Wavelength corresponding to slice, in units of spectrum"
></v-text-field>
</v-col>
<v-col cols=3>
<span>{{ wavelength_unit }}</span>
</v-col>
<v-row>
<v-text-field
v-model="value"
class="mt-0 pt-0"
:label="value_label"
:hint="value_label+' corresponding to slice'"
:suffix="value_unit"
></v-text-field>
</v-row>

<v-row class="row-no-outside-padding row-min-bottom-padding">
Expand Down
12 changes: 6 additions & 6 deletions jdaviz/configs/cubeviz/plugins/slice/tests/test_slice.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ def test_slice(cubeviz_helper, spectrum1d_cube):
assert sl.slice == 1

# from the widget this logic is duplicated (to avoid sending logic through messages)
sl._on_wavelength_updated({'new': '4.62e-07'})
sl._on_value_updated({'new': '4.62e-07'})
assert sl.slice == 0
assert np.allclose(sl.wavelength, 4.62280007e-07)
assert np.allclose(sl.value, 4.62280007e-07)

# make sure that passing an invalid value from the UI would revert to the previous value
# JS strips invalid characters, but doesn't ensure its float-compatible
sl._on_wavelength_updated({'new': '1.2.3'})
sl._on_value_updated({'new': '1.2.3'})
assert sl.slice == 0

assert len(sl._watched_viewers) == 2 # flux-viewer, uncert-viewer
Expand Down Expand Up @@ -81,7 +81,7 @@ def test_slice(cubeviz_helper, spectrum1d_cube):
assert sl.slice == 0

sl.vue_goto_last()
assert sl.slice == sl.max_value
assert sl.slice == sl.max_slice

sl.vue_play_next() # Should automatically wrap to beginning
assert sl.slice == 0
Expand Down Expand Up @@ -109,13 +109,13 @@ def test_indicator_settings(cubeviz_helper, spectrum1d_cube):

assert sl.show_indicator is True
assert indicator._show_if_inactive is True
assert sl.show_wavelength is True
assert sl.show_value is True
assert indicator.label.visible is True

sl.show_indicator = False
assert indicator._show_if_inactive is False

sl.show_wavelength = False
sl.show_value = False
assert indicator.label.visible is False


Expand Down
Loading