Display units line analysis

[kecnry]

Description

This pull request updates the line analysis plugin to support changes to the display unit.

Screen.Recording.2023-04-04.at.1.36.26.PM.mov

(Note that the limits resetting are reported in #2126)

Changelog will point to #2127 so is ignored from CI here.

Change log entry

• Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
  list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
  should add a no-changelog-entry-needed label.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

• Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
• Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
• Do the proposed changes follow the STScI Style Guides?
• Are tests added/updated as required? If so, do they follow the STScI Style Guides?
• Are docs added/updated as required? If so, do they follow the STScI Style Guides?
• Did the CI pass? If not, are the failures related?
• Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone.
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

[javerbukh]

Works well, thanks!

[duytnguyendtn]

Specviz works as the demo video shows on my end! Though I did run into this in Cubeviz example notebook. Confirmed this isn't on main for me. Anyone else seeing this? Should i report this in #2127?

[kecnry]

Adding support for cubeviz and the slice plugin is definitely on the todo list, so I added a todo item in the description of #2127 pointing back here (I don't remember seeing this on my machine, but it could be data or update-version dependent).

[duytnguyendtn]

One more error to note in Mosviz (found in MosvizNIRISSExample). I'll add it to the list

ValueError: cannot detect unit conversion plugin

Traceback

>>> mosviz.load_data(directory=f"{data_dir}/niriss_1324_reprocessed/", instrument="niriss")
Loading: Images
Loading: 2D Spectra C sources
Loading: 2D Spectra R sources
Loading: 1D Spectra C sources
Loading: 1D Spectra R sources
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[5], line 1
----> 1 mosviz.load_data(directory=f"{data_dir}/niriss_1324_reprocessed/", instrument="niriss")

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\configs\mosviz\helper.py:534, in Mosviz.load_data(self, spectra_1d, spectra_2d, images, spectra_1d_label, spectra_2d_label, images_label, *args, **kwargs)
    529 self.app.get_viewer(
    530     self._default_image_viewer_reference_name
    531 ).figure.axes[1].label_offset = "-50"
    533 # Load the first object into the viewers automatically
--> 534 self.app.get_viewer(self._default_table_viewer_reference_name).figure_widget.highlighted = 0
    536 # Notify the user that this all loaded successfully
    537 loaded_msg = SnackbarMessage("MOS data loaded successfully", color="success", sender=self)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\traitlets\traitlets.py:732, in TraitType.__set__(self, obj, value)
    730     raise TraitError('The "%s" trait is read-only.' % self.name)
    731 else:
--> 732     self.set(obj, value)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\traitlets\traitlets.py:721, in TraitType.set(self, obj, value)
    717     silent = False
    718 if silent is not True:
    719     # we explicitly compare silent to True just in case the equality
    720     # comparison above returns something other than True/False
--> 721     obj._notify_trait(self.name, old_value, new_value)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\traitlets\traitlets.py:1505, in HasTraits._notify_trait(self, name, old_value, new_value)
   1504 def _notify_trait(self, name, old_value, new_value):
-> 1505     self.notify_change(
   1506         Bunch(
   1507             name=name,
   1508             old=old_value,
   1509             new=new_value,
   1510             owner=self,
   1511             type="change",
   1512         )
   1513     )

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\ipywidgets\widgets\widget.py:701, in Widget.notify_change(self, change)
    698     if name in self.keys and self._should_send_property(name, getattr(self, name)):
    699         # Send new state to front-end
    700         self.send_state(key=name)
--> 701 super().notify_change(change)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\traitlets\traitlets.py:1517, in HasTraits.notify_change(self, change)
   1515 def notify_change(self, change):
   1516     """Notify observers of a change event"""
-> 1517     return self._notify_observers(change)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\traitlets\traitlets.py:1564, in HasTraits._notify_observers(self, event)
   1561 elif isinstance(c, EventHandler) and c.name is not None:
   1562     c = getattr(self, c.name)
-> 1564 c(event)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\configs\mosviz\plugins\viewers.py:228, in MosvizTableViewer._on_row_selected(self, event)
    222                     self.session.hub.broadcast(remove_data_from_viewer_message)
    224         add_data_to_viewer_message = AddDataToViewerMessage(
    225             self._default_spectrum_viewer_reference_name,
    226             selected_data, sender=self
    227         )
--> 228         self.session.hub.broadcast(add_data_to_viewer_message)
    230         self._selected_data[
    231             self._default_spectrum_viewer_reference_name
    232         ] = selected_data
    234 if component.label == '2D Spectra':

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\glue\core\hub.py:215, in Hub.broadcast(self, message)
    213 logging.getLogger(__name__).info("Broadcasting %s", message)
    214 for subscriber, handler in self._find_handlers(message):
--> 215     handler(message)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\app.py:296, in Application.__init__.<locals>.<lambda>(msg)
    290 # Subscribe to the event fired when data is deleted from the
    291 #  application-level data collection object
    292 self.hub.subscribe(self, DataCollectionDeleteMessage,
    293                    handler=self._on_data_deleted)
    295 self.hub.subscribe(self, AddDataToViewerMessage,
--> 296                    handler=lambda msg: self.add_data_to_viewer(
    297                        msg.viewer_reference, msg.data_label))
    299 self.hub.subscribe(self, RemoveDataFromViewerMessage,
    300                    handler=lambda msg: self.remove_data_from_viewer(
    301                        msg.viewer_reference, msg.data_label))
    303 # Subscribe to snackbar messages and tie them to the display of the
    304 #  message box

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\app.py:1263, in Application.add_data_to_viewer(self, viewer_reference, data_label, visible, clear_other_data)
   1260 if viewer_item is None:
   1261     raise ValueError(f"Could not identify viewer with reference {viewer_reference}")
-> 1263 self.set_data_visibility(viewer_item, data_label, visible=visible, replace=clear_other_data)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\app.py:1606, in Application.set_data_visibility(self, viewer_reference, data_label, visible, replace)
   1601     viewer.add_data(data, percentile=95, color=viewer.color_cycler())
   1603     add_data_message = AddDataMessage(data, viewer,
   1604                                       viewer_id=viewer_id,
   1605                                       sender=self)
-> 1606     self.hub.broadcast(add_data_message)
   1608 # set visibility state of all applicable layers
   1609 for layer in viewer.layers:

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\glue\core\hub.py:215, in Hub.broadcast(self, message)
    213 logging.getLogger(__name__).info("Broadcasting %s", message)
    214 for subscriber, handler in self._find_handlers(message):
--> 215     handler(message)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\core\template_mixin.py:1606, in DatasetSelect._on_data_changed(self, msg)
   1602 if getattr(self, '_include_spatial_subsets', False):
   1603     # allow for spatial subsets to be listed
   1604     self.items = self.items + [_dc_to_dict(subset) for subset in self.app.data_collection.subset_groups  # noqa
   1605                                if _subset_type(subset) == 'spatial']
-> 1606 self._apply_default_selection()
   1607 # future improvement: only clear cache if the selected data entry was changed?
   1608 self._clear_cache(*self._cached_properties)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\core\template_mixin.py:608, in SelectPluginComponent._apply_default_selection(self, skip_if_current_valid)
    605     return
    607 if self.default_mode == 'first':
--> 608     self.selected = self.labels[0] if len(self.labels) else ''
    609 elif self.default_mode == 'default_text':
    610     self.selected = self._default_text if self._default_text else ''

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\core\template_mixin.py:343, in BasePluginComponent.__setattr__(self, attr, value, force_super)
    340 if attr[0] == '_' or force_super or attr not in self._plugin_traitlets.keys():
    341     return super().__setattr__(attr, value)
--> 343 return setattr(self._plugin, self._plugin_traitlets.get(attr), value)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\traitlets\traitlets.py:732, in TraitType.__set__(self, obj, value)
    730     raise TraitError('The "%s" trait is read-only.' % self.name)
    731 else:
--> 732     self.set(obj, value)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\traitlets\traitlets.py:721, in TraitType.set(self, obj, value)
    717     silent = False
    718 if silent is not True:
    719     # we explicitly compare silent to True just in case the equality
    720     # comparison above returns something other than True/False
--> 721     obj._notify_trait(self.name, old_value, new_value)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\traitlets\traitlets.py:1505, in HasTraits._notify_trait(self, name, old_value, new_value)
   1504 def _notify_trait(self, name, old_value, new_value):
-> 1505     self.notify_change(
   1506         Bunch(
   1507             name=name,
   1508             old=old_value,
   1509             new=new_value,
   1510             owner=self,
   1511             type="change",
   1512         )
   1513     )

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\ipywidgets\widgets\widget.py:701, in Widget.notify_change(self, change)
    698     if name in self.keys and self._should_send_property(name, getattr(self, name)):
    699         # Send new state to front-end
    700         self.send_state(key=name)
--> 701 super().notify_change(change)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\traitlets\traitlets.py:1517, in HasTraits.notify_change(self, change)
   1515 def notify_change(self, change):
   1516     """Notify observers of a change event"""
-> 1517     return self._notify_observers(change)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\traitlets\traitlets.py:1564, in HasTraits._notify_observers(self, event)
   1561 elif isinstance(c, EventHandler) and c.name is not None:
   1562     c = getattr(self, c.name)
-> 1564 c(event)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\configs\default\plugins\model_fitting\model_fitting.py:331, in ModelFitting._dataset_selected_changed(self, event)
    327 if not hasattr(self, 'dataset') or self.app._jdaviz_helper is None or self.dataset_selected == '':  # noqa
    328     # during initial init, this can trigger before the component is initialized
    329     return
--> 331 selected_spec = self._get_1d_spectrum()
    332 if selected_spec is None:
    333     return

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\configs\default\plugins\model_fitting\model_fitting.py:310, in ModelFitting._get_1d_spectrum(self)
    308 def _get_1d_spectrum(self):
    309     # retrieves the 1d spectrum (accounting for spatial subset for cubeviz, if necessary)
--> 310     return self.dataset.selected_spectrum_for_spatial_subset(self.spatial_subset_selected)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\core\template_mixin.py:1540, in DatasetSelect.selected_spectrum_for_spatial_subset(self, spatial_subset, use_display_units)
   1538 if spatial_subset == SPATIAL_DEFAULT_TEXT:
   1539     spatial_subset = None
-> 1540 return self.plugin._specviz_helper.get_data(data_label=self.selected,
   1541                                             subset_to_apply=spatial_subset,
   1542                                             use_display_units=use_display_units)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\configs\specviz\helper.py:306, in Specviz.get_data(self, data_label, cls, subset_to_apply, use_display_units)
    303 else:
    304     function = None
--> 306 return self._get_data(data_label=data_label, cls=cls, subset_to_apply=subset_to_apply,
    307                       function=function, use_display_units=use_display_units)

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\core\helpers.py:473, in ConfigHelper._get_data(self, data_label, cls, subset_to_apply, function, use_display_units)
    470     else:
    471         data = data.get_object(cls=cls, **object_kwargs)
--> 473     return _handle_display_units(data, use_display_units)
    475 if not cls and subset_to_apply:
    476     raise AttributeError(f"A valid cls must be provided to"
    477                          f" apply subset {subset_to_apply} to data. "
    478                          f"Instead, {cls} was given.")

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\core\helpers.py:415, in ConfigHelper._get_data.<locals>._handle_display_units(data, use_display_units)
    413 if use_display_units:
    414     if isinstance(data, Spectrum1D):
--> 415         spectral_unit = self.app._get_display_unit('spectral')
    416         flux_unit = self.app._get_display_unit('flux')
    417         # TODO: any other attributes (meta, wcs, etc)?
    418         # TODO: implement uncertainty.to upstream

File E:\STScI\gitRepos\jdaviz\envpr2128\lib\site-packages\jdaviz\app.py:1078, in Application._get_display_unit(self, axis)
   1076 def _get_display_unit(self, axis):
   1077     if self._jdaviz_helper is None or self._jdaviz_helper.plugins.get('Unit Conversion') is None:  # noqa
-> 1078         raise ValueError("cannot detect unit conversion plugin")
   1079     try:
   1080         return getattr(self._jdaviz_helper.plugins.get('Unit Conversion')._obj,
   1081                        f'{axis}_unit_selected')

ValueError: cannot detect unit conversion plugin

[duytnguyendtn]

Okay with the issues in Cubeviz and Mosviz noted and squarely looking at Specviz, the reported behavior does work for me! A quick runthrough of the code looks good, though I'm not going through with a fine-toothed comb for now; figured we do that at the end once we merge the whole branch in