Skip to content

Commit

Permalink
display units: fix failing tests (#2132)
Browse files Browse the repository at this point in the history
* fix error raised when spectrum uncertainty is None
* fix RTD build failure because of missing mixin available in import
* fix test setting display unit now that um is preferred to micron
* add disabled unit conversion plugin to mosviz
* avoid unit-conversion error in cubeviz tests
* bump versions of glue/glue-jupyter
* fix deg intialization in cubeviz slice plugin
* add LinesAutoUnit to __all__ so RTD can find it
* add use_display_units kwarg to get_spectral_regions
* fix setting display units for unitless data
* Apply suggestions from code review

Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>
  • Loading branch information
kecnry and pllim committed Apr 18, 2023
1 parent 7e48396 commit 0ed32f7
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 17 deletions.
3 changes: 2 additions & 1 deletion jdaviz/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,8 @@ def get_subsets(self, subset_name=None, spectral_only=False,
simplify_spectral : bool
Return a composite spectral subset collapsed to a simplified SpectralRegion.
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 <unit-conversion>` plugin.
Returns
-------
Expand Down
3 changes: 2 additions & 1 deletion jdaviz/configs/cubeviz/plugins/slice/slice.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()))

Expand Down
1 change: 1 addition & 0 deletions jdaviz/configs/mosviz/mosviz.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions jdaviz/configs/specviz/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,18 +128,24 @@ 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
:ref:`Unit Conversion <unit-conversion>` 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
Expand Down
9 changes: 6 additions & 3 deletions jdaviz/configs/specviz/plugins/viewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 else u.dimensionless_unscaled
if x_unit.is_equivalent(u.m):
spectral_axis_unit_type = "Wavelength"
elif x_unit.is_equivalent(u.Hz):
Expand Down
16 changes: 10 additions & 6 deletions jdaviz/configs/specviz/tests/test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,17 +293,21 @@ def test_get_spectral_regions_unit_conversion(specviz_helper, spectrum1d):
assert label_mouseover.icon == ''

# Convert the wavelength axis to micron
new_spectral_axis = "micron"
spec_viewer.state.x_display_unit = new_spectral_axis
spec_viewer.set_plot_axes()
new_spectral_axis = "um"
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,
Expand Down
9 changes: 8 additions & 1 deletion jdaviz/core/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,13 +415,20 @@ 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
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
Expand Down
2 changes: 1 addition & 1 deletion jdaviz/core/marks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down
1 change: 1 addition & 0 deletions jdaviz/core/template_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@


__all__ = ['show_widget', 'TemplateMixin', 'PluginTemplateMixin',
'ViewerPropertiesMixin',
'BasePluginComponent', 'SelectPluginComponent', 'UnitSelectPluginComponent',
'PluginSubcomponent',
'SubsetSelect', 'SpatialSubsetSelectMixin', 'SpectralSubsetSelectMixin',
Expand Down
4 changes: 2 additions & 2 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 0ed32f7

Please sign in to comment.