From 3b5fea7463ecdea0ad746b1aca0c2799c06ae24a Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 5 Apr 2023 09:27:38 -0400 Subject: [PATCH 1/8] bump glue-core to necessary version for display units --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 11508b190a..7d0acb9155 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,7 @@ dependencies = [ "traitlets>=5.0.5", "bqplot>=0.12.37", "bqplot-image-gl>=1.4.11", - "glue-core>=1.10.1", + "glue-core>=1.10.0", "glue-jupyter>=0.16.3", "echo>=0.5.0", "ipykernel>=6.19.4", From 7dc211f8a3a7ebc0038e88e1a5044f3dd8dcb4af Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 5 Apr 2023 09:30:22 -0400 Subject: [PATCH 2/8] cubeviz/slice plugin unit conversion support (we might want to generalize some of this logic more into the mark's _update_data when adding support for spectral lines as well) --- jdaviz/core/helpers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index 8939b0d876..4cc677cbce 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -423,8 +423,7 @@ def _handle_display_units(data, use_display_units): # 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') + 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 From 8a242f9d0109de2f144edd876495553716a0b395 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 13 Apr 2023 15:22:52 -0400 Subject: [PATCH 3/8] display units: fix failing tests (#2132) * 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> --- jdaviz/configs/mosviz/mosviz.yaml | 1 + jdaviz/core/helpers.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) 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 diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index 4cc677cbce..8939b0d876 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -423,7 +423,8 @@ def _handle_display_units(data, use_display_units): # 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') + 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 From 2f5e25acb6b3f87e4701b3919566b38de1963016 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 4 May 2023 09:19:41 -0400 Subject: [PATCH 4/8] remove unit conversion plugin from all bug specviz * for initial implementation - eventually we'll want to implement across all plugins/viewers --- jdaviz/configs/mosviz/mosviz.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/jdaviz/configs/mosviz/mosviz.yaml b/jdaviz/configs/mosviz/mosviz.yaml index 98f842b08b..b5a846350c 100644 --- a/jdaviz/configs/mosviz/mosviz.yaml +++ b/jdaviz/configs/mosviz/mosviz.yaml @@ -22,7 +22,6 @@ tray: - g-gaussian-smooth - g-slit-overlay - g-model-fitting - - g-unit-conversion - g-line-list - specviz-line-analysis - g-export-plot From 945d232539f192a3e1619629201951c8575c51a1 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Wed, 10 May 2023 17:03:28 -0400 Subject: [PATCH 5/8] Working on unit conversion handling in subset plugin --- .../default/plugins/subset_plugin/subset_plugin.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py index 3af7db91f5..e04f846cf1 100644 --- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py +++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py @@ -1,4 +1,5 @@ import numpy as np +import astropy.units as u from glue.core.message import EditSubsetMessage, SubsetUpdateMessage from glue.core.edit_subset_mode import (AndMode, AndNotMode, OrMode, ReplaceMode, XorMode) @@ -7,7 +8,7 @@ from glue_jupyter.widgets.subset_mode_vuetify import SelectionModeMenu from traitlets import Any, List, Unicode, Bool, observe -from jdaviz.core.events import SnackbarMessage +from jdaviz.core.events import SnackbarMessage, GlobalDisplayUnitChanged from jdaviz.core.registries import tray_registry from jdaviz.core.template_mixin import PluginTemplateMixin, DatasetSelectMixin, SubsetSelect @@ -52,6 +53,8 @@ def __init__(self, *args, **kwargs): handler=self._sync_selected_from_state) self.session.hub.subscribe(self, SubsetUpdateMessage, handler=self._on_subset_update) + self.session.hub.subscribe(self, GlobalDisplayUnitChanged, + handler=self._on_display_unit_changed) self.subset_select = SubsetSelect(self, 'subset_items', @@ -199,6 +202,11 @@ def _get_subset_definition(self, *args): self._unpack_get_subsets_for_ui() + def _on_display_unit_changed(self, msg): + # We only care about the spectral units, since flux units don't affect spectral subsets + if msg.axis == "spectral": + self.spectral_display_unit = msg.unit + def vue_update_subset(self, *args): for index, sub in enumerate(self.subset_definitions): sub_states = self.subset_states[index] From 5e818f2feb37cdd6bf303b39060e1eb25da15788 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 11 May 2023 16:05:49 -0400 Subject: [PATCH 6/8] Get subset updating working in non-default units --- .../plugins/subset_plugin/subset_plugin.py | 27 +++++++++++++++---- .../plugins/subset_plugin/subset_plugin.vue | 1 + 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py index e04f846cf1..9bc7364f0d 100644 --- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py +++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py @@ -61,6 +61,7 @@ def __init__(self, *args, **kwargs): 'subset_selected', default_text="Create New") self.subset_states = [] + self.spectral_display_unit = None def _sync_selected_from_state(self, *args): if not hasattr(self, 'subset_select'): @@ -130,7 +131,8 @@ def _unpack_get_subsets_for_ui(self): ------- """ - subset_information = self.app.get_subsets(self.subset_selected, simplify_spectral=False) + subset_information = self.app.get_subsets(self.subset_selected, simplify_spectral=False, + use_display_units=True) _around_decimals = 6 # Avoid 30 degrees from coming back as 29.999999999999996 if not subset_information: return @@ -178,10 +180,12 @@ def _unpack_get_subsets_for_ui(self): subset_type = subset_state.roi.__class__.__name__ elif isinstance(subset_state, RangeSubsetState): - lo = subset_state.lo - hi = subset_state.hi - subset_definition = [{"name": "Lower bound", "att": "lo", "value": lo, "orig": lo}, - {"name": "Upper bound", "att": "hi", "value": hi, "orig": hi}] + lo = spec['region'].lower + hi = spec['region'].upper + subset_definition = [{"name": "Lower bound", "att": "lo", "value": lo.value, + "orig": lo.value, "unit": str(lo.unit)}, + {"name": "Upper bound", "att": "hi", "value": hi.value, + "orig": hi.value, "unit": str(hi.unit)}] subset_type = "Range" if len(subset_definition) > 0: # Note: .append() does not work for List traitlet. @@ -206,6 +210,8 @@ def _on_display_unit_changed(self, msg): # We only care about the spectral units, since flux units don't affect spectral subsets if msg.axis == "spectral": self.spectral_display_unit = msg.unit + if self.subset_selected != self.subset_select.default_text: + self._get_subset_definition(self.subset_selected) def vue_update_subset(self, *args): for index, sub in enumerate(self.subset_definitions): @@ -215,6 +221,17 @@ def vue_update_subset(self, *args): d_val = np.radians(d_att["value"]) else: d_val = float(d_att["value"]) + + # Convert from display unit to original unit if necessary + if self.subset_types[index] == "Range": + if self.spectral_display_unit is not None: + x_att = sub_states.att + base_units = self.app.data_collection[0].get_component(x_att).units + if self.spectral_display_unit != base_units: + d_val = d_val*u.Unit(self.spectral_display_unit) + d_val = d_val.to(u.Unit(base_units)) + d_val = d_val.value + if float(d_att["orig"]) != d_val: if self.subset_types[index] == "Range": setattr(sub_states, d_att["att"], d_val) diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue index 354c337611..649bc72ae1 100644 --- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue +++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue @@ -59,6 +59,7 @@ v-model.number="item.value" type="number" :disabled="!is_editable" + :suffix="item.unit" > From b0c3799e2cb2f02dd2c26f705deb3aeccd2d928c Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Thu, 11 May 2023 16:12:19 -0400 Subject: [PATCH 7/8] Add changelog --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 1ec8c85d70..41376347f2 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -14,6 +14,8 @@ New Features - Histogram showing image values in stretch limits section of plot options plugin. [#2153] +- Subset Plugin now respects the chosen display unit after using Unit Conversion. [#2195] + Cubeviz ^^^^^^^ From a22e6181a00117d59eff03c117681d0b4de12a9d Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <39831871+rosteen@users.noreply.github.com> Date: Mon, 15 May 2023 15:19:35 -0400 Subject: [PATCH 8/8] Update jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue Co-authored-by: Kyle Conroy --- jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue index 649bc72ae1..f833ce7644 100644 --- a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue +++ b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue @@ -59,7 +59,7 @@ v-model.number="item.value" type="number" :disabled="!is_editable" - :suffix="item.unit" + :suffix="item.unit.replace('Angstrom', 'A')" >