From c87929631838d30283042e631f1e893373966b39 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 5 Apr 2023 14:51:14 -0400 Subject: [PATCH 01/12] fix error raised when spectrum uncertainty is None --- jdaviz/core/helpers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index cda5e91d20..5b988a1a89 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -418,10 +418,11 @@ def _handle_display_units(data, use_display_units): flux_unit = self.app._get_display_unit('flux') # TODO: any other attributes (meta, wcs, etc)? # TODO: implement uncertainty.to upstream + new_uncert = data.uncertainty.__class__(data.uncertainty.quantity.to(flux_unit)) if data.uncertainty is not None else None # noqa data = Spectrum1D(spectral_axis=data.spectral_axis.to(spectral_unit, u.spectral()), flux=data.flux.to(flux_unit), - uncertainty=data.uncertainty.__class__(data.uncertainty.quantity.to(flux_unit))) # noqa + uncertainty=new_uncert) else: # pragma: nocover raise NotImplementedError(f"converting {data.__class__.__name__} to display units is not supported") # noqa return data From 970836dd208162448284ee4ec9a72b26f7c8bfe8 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 5 Apr 2023 14:51:32 -0400 Subject: [PATCH 02/12] fix RTD build failure because of missing mixin available in import --- jdaviz/core/template_mixin.py | 1 + 1 file changed, 1 insertion(+) diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 4092724239..ded3170d26 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -28,6 +28,7 @@ __all__ = ['show_widget', 'TemplateMixin', 'PluginTemplateMixin', + 'ViewerPropertiesMixin', 'BasePluginComponent', 'SelectPluginComponent', 'UnitSelectPluginComponent', 'PluginSubcomponent', 'SubsetSelect', 'SpatialSubsetSelectMixin', 'SpectralSubsetSelectMixin', From c1e19b7a7ee7aff9eea687b95758f30e37b4d1f9 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 5 Apr 2023 14:53:52 -0400 Subject: [PATCH 03/12] fix test setting display unit now that um is preferred to micron --- jdaviz/configs/specviz/tests/test_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/configs/specviz/tests/test_helper.py b/jdaviz/configs/specviz/tests/test_helper.py index f3041ab734..2be9d6cb9c 100644 --- a/jdaviz/configs/specviz/tests/test_helper.py +++ b/jdaviz/configs/specviz/tests/test_helper.py @@ -293,7 +293,7 @@ def test_get_spectral_regions_unit_conversion(specviz_helper, spectrum1d): assert label_mouseover.icon == '' # Convert the wavelength axis to micron - new_spectral_axis = "micron" + new_spectral_axis = "um" spec_viewer.state.x_display_unit = new_spectral_axis spec_viewer.set_plot_axes() From b4e5d3e7f0cc46d1446bf2a1131af806a431fbd1 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 5 Apr 2023 15:06:18 -0400 Subject: [PATCH 04/12] add disabled unit conversion plugin to mosviz --- jdaviz/configs/mosviz/mosviz.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/jdaviz/configs/mosviz/mosviz.yaml b/jdaviz/configs/mosviz/mosviz.yaml index b5a846350c..98f842b08b 100644 --- a/jdaviz/configs/mosviz/mosviz.yaml +++ b/jdaviz/configs/mosviz/mosviz.yaml @@ -22,6 +22,7 @@ tray: - g-gaussian-smooth - g-slit-overlay - g-model-fitting + - g-unit-conversion - g-line-list - specviz-line-analysis - g-export-plot From f6058f2fc93ac6da8a7e19a40dd42add32bd0503 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 5 Apr 2023 15:15:35 -0400 Subject: [PATCH 05/12] avoid unit-conversion error in cubeviz tests --- jdaviz/core/helpers.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index 5b988a1a89..ed353d141c 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -415,6 +415,12 @@ def _handle_display_units(data, use_display_units): spectral_unit = self.app._get_display_unit('spectral') if not spectral_unit: return data + if self.app.config == 'cubeviz' and spectral_unit == 'deg': + # this happens before the correct axis is set for the spectrum-viewer + # and would result in a unit-conversion error if attempting to convert + # to the display units. This should only ever be temporary during + # app intialization. + return data flux_unit = self.app._get_display_unit('flux') # TODO: any other attributes (meta, wcs, etc)? # TODO: implement uncertainty.to upstream From 83994c83c515475946d53420f82c6c8b94c37765 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 13 Apr 2023 09:17:23 -0400 Subject: [PATCH 06/12] bump versions of glue/glue-jupyter --- setup.cfg | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.cfg b/setup.cfg index 575c6555c2..6e03f9b7c6 100644 --- a/setup.cfg +++ b/setup.cfg @@ -24,8 +24,8 @@ install_requires = traitlets>=5.0.5 bqplot>=0.12.37 bqplot-image-gl>=1.4.11 - glue-core>=1.9.0 - glue-jupyter>=0.15.0 + glue-core>=1.9.1 + glue-jupyter>=0.16.2 echo>=0.5.0 ipykernel>=6.19.4 ipyvue>=1.6 From 36901433758c13179e4f6f5089788213e3621a75 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 13 Apr 2023 13:06:18 -0400 Subject: [PATCH 07/12] fix deg intialization in cubeviz slice plugin --- jdaviz/configs/cubeviz/plugins/slice/slice.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jdaviz/configs/cubeviz/plugins/slice/slice.py b/jdaviz/configs/cubeviz/plugins/slice/slice.py index 57026f9d63..0ff906af8c 100644 --- a/jdaviz/configs/cubeviz/plugins/slice/slice.py +++ b/jdaviz/configs/cubeviz/plugins/slice/slice.py @@ -171,7 +171,8 @@ def _on_global_display_unit_changed(self, msg): return prev_unit = self.wavelength_unit self.wavelength_unit = msg.unit.to_string() - if self._x_all is None or prev_unit == '': + # 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())) From ee12c73d5a7bf980ab2c74c9fe514775dd193fc3 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 13 Apr 2023 13:10:54 -0400 Subject: [PATCH 08/12] add LinesAutoUnit to __all__ so RTD can find it --- jdaviz/core/marks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index 08ffba76b9..95b29843f0 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -14,7 +14,7 @@ __all__ = ['OffscreenLinesMarks', 'BaseSpectrumVerticalLine', 'SpectralLine', 'SliceIndicatorMarks', 'ShadowMixin', 'ShadowLine', 'ShadowLabelFixedY', - 'PluginMark', 'PluginLine', 'PluginScatter', + 'PluginMark', 'LinesAutoUnit', 'PluginLine', 'PluginScatter', 'LineAnalysisContinuum', 'LineAnalysisContinuumCenter', 'LineAnalysisContinuumLeft', 'LineAnalysisContinuumRight', 'LineUncertainties', 'ScatterMask', 'SelectedSpaxel', 'MarkersMark'] From c4bb23d9b6a666126f178071f0f47fde65caf183 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 13 Apr 2023 13:18:23 -0400 Subject: [PATCH 09/12] add use_display_units kwarg to get_spectral_regions --- jdaviz/configs/specviz/helper.py | 9 +++++++-- jdaviz/configs/specviz/tests/test_helper.py | 14 +++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/jdaviz/configs/specviz/helper.py b/jdaviz/configs/specviz/helper.py index 7692ec04be..e3cd24495b 100644 --- a/jdaviz/configs/specviz/helper.py +++ b/jdaviz/configs/specviz/helper.py @@ -128,18 +128,23 @@ def get_spectra(self, data_label=None, subset_to_apply=None, apply_slider_redshi return output_spectra - def get_spectral_regions(self): + def get_spectral_regions(self, use_display_units=False): """ A simple wrapper around the app-level call to retrieve only spectral subsets, which are now returned as SpectralRegions by default. + Parameters + ---------- + use_display_units: bool, optional + Whether to convert to the display units defined in the plugin. + Returns ------- spec_regs : dict Mapping from the names of the subsets to the subsets expressed as `specutils.SpectralRegion` objects. """ - return self.app.get_subsets(spectral_only=True) + return self.app.get_subsets(spectral_only=True, use_display_units=use_display_units) def x_limits(self, x_min=None, x_max=None): """Sets the limits of the x-axis diff --git a/jdaviz/configs/specviz/tests/test_helper.py b/jdaviz/configs/specviz/tests/test_helper.py index 2be9d6cb9c..53e8122dbd 100644 --- a/jdaviz/configs/specviz/tests/test_helper.py +++ b/jdaviz/configs/specviz/tests/test_helper.py @@ -294,16 +294,20 @@ def test_get_spectral_regions_unit_conversion(specviz_helper, spectrum1d): # Convert the wavelength axis to micron new_spectral_axis = "um" - spec_viewer.state.x_display_unit = new_spectral_axis - spec_viewer.set_plot_axes() + specviz_helper.plugins['Unit Conversion'].spectral_unit = new_spectral_axis spec_viewer.apply_roi(XRangeROI(0.6, 0.7)) # Retrieve the Subset - subsets = specviz_helper.get_spectral_regions() + subsets = specviz_helper.get_spectral_regions(use_display_units=False) + reg = subsets.get('Subset 1') + assert reg.lower.unit == u.Angstrom + assert reg.upper.unit == u.Angstrom + + subsets = specviz_helper.get_spectral_regions(use_display_units=True) reg = subsets.get('Subset 1') - assert reg.lower.unit == u.micron - assert reg.upper.unit == u.micron + assert reg.lower.unit == u.um + assert reg.upper.unit == u.um # Coordinates info panel should show new unit label_mouseover._viewer_mouse_event(spec_viewer, From a41c92f8012eb987d537eb3ddfd653b0cfb3cbbb Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 13 Apr 2023 13:33:40 -0400 Subject: [PATCH 10/12] fix setting display units for unitless data --- jdaviz/configs/specviz/plugins/viewers.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/viewers.py b/jdaviz/configs/specviz/plugins/viewers.py index a917fb4578..7b77e93d6b 100644 --- a/jdaviz/configs/specviz/plugins/viewers.py +++ b/jdaviz/configs/specviz/plugins/viewers.py @@ -361,8 +361,10 @@ def add_data(self, data, color=None, alpha=None, **layer_state): result = super().add_data(data, color, alpha, **layer_state) if reset_plot_axes: - self.state.x_display_unit = data.get_component(self.state.x_att.label).units - self.state.y_display_unit = data.get_component("flux").units + x_units = data.get_component(self.state.x_att.label).units + y_units = data.get_component("flux").units + self.state.x_display_unit = x_units if len(x_units) else None + self.state.y_display_unit = y_units if len(y_units) else None self.set_plot_axes() self._plot_uncertainties() @@ -489,7 +491,8 @@ def _plot_uncertainties(self): def set_plot_axes(self): # Set axes labels for the spectrum viewer flux_unit_type = "Flux density" - x_unit = u.Unit(self.state.x_display_unit) + x_disp_unit = self.state.x_display_unit + x_unit = u.Unit(x_disp_unit) if x_disp_unit is not None else u.dimensionless_unscaled if x_unit.is_equivalent(u.m): spectral_axis_unit_type = "Wavelength" elif x_unit.is_equivalent(u.Hz): From f030cce1c140a2ebee4615ff93913bf28b1f34a5 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 13 Apr 2023 14:53:56 -0400 Subject: [PATCH 11/12] Apply suggestions from code review Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> --- jdaviz/app.py | 4 ++-- jdaviz/configs/cubeviz/plugins/slice/slice.py | 2 +- jdaviz/configs/specviz/helper.py | 4 ++-- jdaviz/configs/specviz/plugins/viewers.py | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index c7341fc606..6e6216cdad 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -886,8 +886,8 @@ def get_subsets(self, subset_name=None, spectral_only=False, object_only : bool Return only object relevant information and leave out the region class name and glue_state. - use_display_units: bool, optional - Whether to convert to the display units defined in the plugin. + use_display_units : bool, optional + Whether to convert to the display units defined in the Unit Conversion plugin. Returns ------- diff --git a/jdaviz/configs/cubeviz/plugins/slice/slice.py b/jdaviz/configs/cubeviz/plugins/slice/slice.py index 0ff906af8c..04ef23a59e 100644 --- a/jdaviz/configs/cubeviz/plugins/slice/slice.py +++ b/jdaviz/configs/cubeviz/plugins/slice/slice.py @@ -172,7 +172,7 @@ def _on_global_display_unit_changed(self, msg): prev_unit = self.wavelength_unit self.wavelength_unit = msg.unit.to_string() # 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', '']: + 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())) diff --git a/jdaviz/configs/specviz/helper.py b/jdaviz/configs/specviz/helper.py index e3cd24495b..ac1cb3d2f6 100644 --- a/jdaviz/configs/specviz/helper.py +++ b/jdaviz/configs/specviz/helper.py @@ -135,8 +135,8 @@ def get_spectral_regions(self, use_display_units=False): Parameters ---------- - use_display_units: bool, optional - Whether to convert to the display units defined in the plugin. + use_display_units : bool, optional + Whether to convert to the display units defined in the Unit Conversion plugin. Returns ------- diff --git a/jdaviz/configs/specviz/plugins/viewers.py b/jdaviz/configs/specviz/plugins/viewers.py index 7b77e93d6b..e1de39a615 100644 --- a/jdaviz/configs/specviz/plugins/viewers.py +++ b/jdaviz/configs/specviz/plugins/viewers.py @@ -492,7 +492,7 @@ def set_plot_axes(self): # Set axes labels for the spectrum viewer flux_unit_type = "Flux density" x_disp_unit = self.state.x_display_unit - x_unit = u.Unit(x_disp_unit) if x_disp_unit is not None else u.dimensionless_unscaled + x_unit = u.Unit(x_disp_unit) if x_disp_unit else u.dimensionless_unscaled if x_unit.is_equivalent(u.m): spectral_axis_unit_type = "Wavelength" elif x_unit.is_equivalent(u.Hz): From 7bda40b1b62864a4e7b5a13dafdf255f0e8610d0 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 13 Apr 2023 15:00:41 -0400 Subject: [PATCH 12/12] fix RTD link in API docstring --- jdaviz/app.py | 3 ++- jdaviz/configs/specviz/helper.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 6e6216cdad..773c5706b7 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -887,7 +887,8 @@ def get_subsets(self, subset_name=None, spectral_only=False, Return only object relevant information and leave out the region class name and glue_state. use_display_units : bool, optional - Whether to convert to the display units defined in the Unit Conversion plugin. + Whether to convert to the display units defined in the + :ref:`Unit Conversion ` plugin. Returns ------- diff --git a/jdaviz/configs/specviz/helper.py b/jdaviz/configs/specviz/helper.py index ac1cb3d2f6..e5252ee985 100644 --- a/jdaviz/configs/specviz/helper.py +++ b/jdaviz/configs/specviz/helper.py @@ -136,7 +136,8 @@ def get_spectral_regions(self, use_display_units=False): Parameters ---------- use_display_units : bool, optional - Whether to convert to the display units defined in the Unit Conversion plugin. + Whether to convert to the display units defined in the + :ref:`Unit Conversion ` plugin. Returns -------