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

fix spectral extraction previews on unit change #3157

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
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: 1 addition & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ New Features
------------

- Added flux/surface brightness translation and surface brightness
unit conversion in Cubeviz and Specviz. [#2781, #2940, #3088, #3111, #3113, #3129, #3139, #3155]
unit conversion in Cubeviz and Specviz. [#2781, #2940, #3088, #3111, #3113, #3129, #3139, #3155, #3157]

- Plugin tray is now open by default. [#2892]

Expand Down
7 changes: 6 additions & 1 deletion jdaviz/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -1274,7 +1274,12 @@ def merge_func(spectral_region): # noop
return self._get_multi_mask_subset_definition(subset_state)

def _get_display_unit(self, axis):
if self._jdaviz_helper is None or self._jdaviz_helper.plugins.get('Unit Conversion') is None: # noqa
if self._jdaviz_helper is None:
# cannot access either the plugin or the spectrum viewer.
# Plugins that access the unit at this point will need to
# detect that they are set to unitless and attempt again later.
return ''
elif self._jdaviz_helper.plugins.get('Unit Conversion') is None: # noqa
# fallback on native units (unit conversion is not enabled)
if axis == 'spectral':
sv = self.get_viewer(self._jdaviz_helper._default_spectrum_viewer_reference_name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from jdaviz.core.user_api import PluginUserApi
from jdaviz.configs.cubeviz.plugins.parsers import _return_spectrum_with_correct_units
from jdaviz.configs.cubeviz.plugins.viewers import WithSliceIndicator
from jdaviz.utils import _eqv_pixar_sr


__all__ = ['SpectralExtraction']
Expand Down Expand Up @@ -109,8 +110,8 @@ class SpectralExtraction(PluginTemplateMixin, ApertureSubsetSelectMixin,

results_units = Unicode().tag(sync=True)
spectrum_y_units = Unicode().tag(sync=True)
flux_unit = Unicode().tag(sync=True)
sb_unit = Unicode().tag(sync=True)
flux_units = Unicode().tag(sync=True)
sb_units = Unicode().tag(sync=True)
Comment on lines -112 to +114
Copy link
Member Author

Choose a reason for hiding this comment

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

just to be consistent with the traitlets above, I was confusing myself when writing code later in the plugin and having to switch between singular and plural


aperture_method_items = List().tag(sync=True)
aperture_method_selected = Unicode('Center').tag(sync=True)
Expand Down Expand Up @@ -180,7 +181,7 @@ def __init__(self, *args, **kwargs):
self.session.hub.subscribe(self, SliceValueUpdatedMessage,
handler=self._on_slice_changed)
self.hub.subscribe(self, GlobalDisplayUnitChanged,
handler=self._on_gloabl_display_unit_changed)
handler=self._on_global_display_unit_changed)

self._update_disabled_msg()

Expand Down Expand Up @@ -315,31 +316,33 @@ def _update_mark_scale(self, *args):
else:
self.background.scale_factor = self.slice_spectral_value/self.reference_spectral_value

def _on_gloabl_display_unit_changed(self, msg={}):
def _on_global_display_unit_changed(self, msg=None):
if msg is None:
self.flux_units = str(self.app._get_display_unit('flux'))
self.sb_units = str(self.app._get_display_unit('sb'))
self.spectrum_y_units = str(self.app._get_display_unit('spectral_y'))

if msg.axis == 'spectral_y':
self.spectrum_y_units = str(msg.unit)

# a 'flux' and 'sb' message should be recieved back to back from
# the unit conversion plugin, so don't need to sync them immediatley
# within each message recieved
elif msg.axis == 'flux':
self.flux_unit = str(msg.unit)
self.flux_units = str(msg.unit)
elif msg.axis == 'sb':
self.sb_unit = str(msg.unit)
self.sb_units = str(msg.unit)
elif msg.axis == 'spectral_y':
self.spectrum_y_units = str(msg.unit)
# no need to update results_units
return
else:
# ignore
return

# and set results_units, which depends on function selected
# update results_units based on flux_units, sb_units, and currently selected function
self._update_units_on_function_selection()

@observe('function_selected')
def _update_units_on_function_selection(self, *args):

if self.function_selected.lower() == 'sum':
self.results_units = self.flux_unit
self.results_units = self.flux_units
else:
self.results_units = self.sb_unit
self.results_units = self.sb_units
# any changes to results_units will trigger _live_update_marks

@observe('function_selected', 'aperture_method_selected')
def _update_aperture_method_on_function_change(self, *args):
Expand Down Expand Up @@ -542,7 +545,9 @@ def _preview_x_from_extracted(self, extracted):
return extracted.spectral_axis.value

def _preview_y_from_extracted(self, extracted):
return extracted.flux.value
# TODO: use extracted's PIXAR_SR instead (but for some reason isn't populated here...)
Copy link
Member Author

Choose a reason for hiding this comment

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

does anyone know why this is (I'm guessing its just populated later when actually loading into the app)? Ultimately, I think the extracted spectrum just gets a copy of PIXAR_SR from the input cube, so this shouldn't matter, it just would read more cleanly if it did.

return extracted.flux.to_value(self.spectrum_y_units,
equivalencies=_eqv_pixar_sr(self.dataset.selected_obj.meta.get('PIXAR_SR', 1.0))) # noqa:

@with_spinner()
def extract(self, return_bg=False, add_data=True, **kwargs):
Expand Down Expand Up @@ -726,8 +731,15 @@ def _clear_marks(self):
'wavelength_dependent', 'bg_wavelength_dependent', 'reference_spectral_value',
'function_selected',
'aperture_method_selected',
'spectrum_y_units', 'results_units',
'previews_temp_disabled')
def _live_update_marks(self, event={}):
if self.spectrum_y_units == '':
# ensure that units are populated
# which in turn will make a call back here
# from the observe on spectrum_y_units
self._on_global_display_unit_changed(None)
return
self._update_marks(event)

@skip_if_not_tray_instance()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def _on_glue_y_display_unit_changed(self, y_unit_str):
if self.app.data_collection[0]:
dc_unit = self.app.data_collection[0].get_component("flux").units
self.angle_unit.choices = create_angle_equivalencies_list(dc_unit)
self.angle_unit.selected = self.angle_unit.choices[0]
self.angle_unit.select_default()
self.sb_unit_selected = self._append_angle_correctly(
self.flux_unit.selected,
self.angle_unit.selected
Expand Down Expand Up @@ -217,8 +217,8 @@ def _on_flux_or_sb_selected(self, msg):
if msg.get('name') == 'flux_or_sb_selected':
self._translate(self.flux_or_sb_selected)

@observe('flux_unit_selected')
def _on_flux_unit_changed(self, msg):
@observe('flux_unit_selected', 'angle_unit_selected')
def _on_flux_or_angle_unit_changed(self, msg):

"""
Observes changes in selected flux unit.
Expand All @@ -231,13 +231,12 @@ def _on_flux_unit_changed(self, msg):
SB is read only, so anything observing for changes in surface brightness
should be looking for a change in 'flux' (as well as angle).
"""

if msg.get('name') != 'flux_unit_selected':
# not sure when this would be encountered but keeping as a safeguard
return
if not hasattr(self, 'flux_unit'):
return
if not self.flux_unit.choices and self.app.config == 'cubeviz':
if not self.flux_unit.choices:
return
if self.flux_unit.selected == '' or self.angle_unit.selected == '':
# wait until flux AND angle unit are both populated during init
return

# various plugins are listening for changes in either flux or sb and
Expand Down
Loading