address Kyle's review comments

spacetelescope · Dec 26, 2024 · 663e158 · 663e158
1 parent 98f2852
commit 663e158
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 23 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -29,7 +29,7 @@ Specviz
 Specviz2d
 ^^^^^^^^^
 
-- Implement the Unit Conversion plugin the Specviz2D. [#3253]
+- Implement the Unit Conversion plugin in Specviz2D. [#3253]
 
 API Changes
 -----------

diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py
@@ -524,10 +524,10 @@ def test_spectral_extraction_with_correct_sum_units(cubeviz_helper,
     assert '_pixel_scale_factor' in collapsed.meta
 
     # Original units in Jy / sr
-    expected_flux_values = [190., 590., 990., 1390., 1790., 2190., 2590., 2990., 3390., 3790.]
     # After collapsing, sr is removed via the scale factor and the extracted spectrum is in Jy
-    expected_flux_values = [flux * collapsed.meta.get('_pixel_scale_factor')
-                            for flux in expected_flux_values]
+    expected_flux_values = (np.array([190., 590., 990., 1390., 1790.,
+                                      2190., 2590., 2990., 3390., 3790.]) *
+                            collapsed.meta.get('_pixel_scale_factor'))
 
     np.testing.assert_allclose(
         collapsed.flux.value,

diff --git a/jdaviz/configs/default/plugins/markers/markers.py b/jdaviz/configs/default/plugins/markers/markers.py
@@ -225,12 +225,6 @@ def _on_is_active_changed(self, *args):
     def _on_viewer_key_event(self, viewer, data):
         if data['event'] == 'keydown' and data['key'] == 'm':
             row_info = self.coords_info.as_dict()
-            if 'value' in row_info:
-                # format and cast flux value for Table
-                row_info['value'] = f"{row_info['value']:.5e}"
-            else:
-                row_info['value'] = ''
-
             if 'viewer' in self.table.headers_avail:
                 row_info['viewer'] = viewer.reference if viewer.reference is not None else viewer.reference_id  # noqa
 

diff --git a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py
@@ -553,7 +553,6 @@ def _initialize_model_component(self, model_comp, comp_label, poly_order=None):
             # equivs for spectral density and flux<>sb
             pixar_sr = masked_spectrum.meta.get('_pixel_scale_factor', 1.0)
             equivs = all_flux_unit_conversion_equivs(pixar_sr, init_x)
-            equivs += u.spectral_density(init_x)
 
             init_y = flux_conversion_general([init_y.value],
                                              init_y.unit,
@@ -905,13 +904,7 @@ def _fit_model_to_spectrum(self, add_data):
             return
         models_to_fit = self._reinitialize_with_fixed()
 
-        spec = self.dataset.selected_spectrum
-        # Needs Attention:
-        # should conversion occur before or after call to _apply_subset_masks?
-        if spec.flux.unit.to_string != self.app._get_display_unit('flux'):
-            equivalencies = u.spectral_density(self.dataset.selected_spectrum.spectral_axis)
-            spec = self.dataset.selected_spectrum.with_flux_unit(self.app._get_display_unit('flux'),
-                                                                 equivalencies=equivalencies)
+        spec = self.dataset.get_selected_spectrum(use_display_units=True)
 
         masked_spectrum = self._apply_subset_masks(spec,
                                                    self.spectral_subset)

diff --git a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py
@@ -559,7 +559,10 @@ def _image_viewer_update(self, viewer, x, y):
             else:
                 dq_text = ''
             self.row1b_text = f'{value:+10.5e} {unit}{dq_text}'
-            self._dict['value'] = float(value)
+            if not isinstance(value, (float, np.floating)):
+                self._dict['value'] = float(value)
+            else:
+                self._dict['value'] = value
             self._dict['value:unit'] = str(unit)
             self._dict['value:unreliable'] = unreliable_pixel
         else:

diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py
@@ -114,8 +114,7 @@ def __init__(self, *args, **kwargs):
         self._cached_properties = ['image_layers']
 
         if self.config not in ['specviz', 'specviz2d', 'cubeviz']:
-            # TODO [specviz2d, mosviz] x_display_unit is not implemented in glue for image viewer
-            # used by spectrum-2d-viewer
+            # TODO [mosviz] x_display_unit is not implemented in glue for image viewer
             # TODO [mosviz]: add to yaml file
             # TODO [cubeviz, slice]: slice indicator broken after changing spectral_unit
             # TODO: support for multiple viewers and handling of mixed state from glue (or does

diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py
@@ -486,7 +486,11 @@ def _handle_display_units(self, data, use_display_units=True):
                                                                eqv=eqv)
                         new_uncert = StdDevUncertainty(new_uncert_converted, unit=y_unit)
                     else:
-                        new_uncert = StdDevUncertainty(new_uncert, unit=data.flux.unit)
+                        eqv = all_flux_unit_conversion_equivs(cube_wave=data.spectral_axis)
+                        new_uncert_converted = flux_conversion(new_uncert.quantity.value,
+                                                               new_uncert.unit, y_unit, spec=data,
+                                                               eqv=eqv)
+                        new_uncert = StdDevUncertainty(new_uncert, unit=y_unit)
 
                 else:
                     new_uncert = None
@@ -497,8 +501,9 @@ def _handle_display_units(self, data, use_display_units=True):
                     new_y = flux_conversion(data.flux.value, data.flux.unit,
                                             y_unit, data, eqv=eqv) * u.Unit(y_unit)
                 else:
+                    eqv = all_flux_unit_conversion_equivs(cube_wave=data.spectral_axis)
                     new_y = flux_conversion(data.flux.value, data.flux.unit,
-                                            data.flux.unit, spec=data) * u.Unit(data.flux.unit)
+                                            y_unit, spec=data) * u.Unit(y_unit)
 
                 new_spec = (spectral_axis_conversion(data.spectral_axis.value,
                                                      data.spectral_axis.unit,