From e9d14507b1228f27785a92250aa31f5c6f2ca1f2 Mon Sep 17 00:00:00 2001 From: Thomas Robitaille Date: Fri, 26 Apr 2024 12:23:28 +0100 Subject: [PATCH 1/3] Fix bug in profile viewer y limits conversion --- glue/viewers/profile/state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glue/viewers/profile/state.py b/glue/viewers/profile/state.py index 7892c3c0b..7d08aff20 100644 --- a/glue/viewers/profile/state.py +++ b/glue/viewers/profile/state.py @@ -146,7 +146,7 @@ def _convert_units_y_limits(self, old_unit, new_unit): layer_state.attribute, limits, old_unit) - limits_new = converter.to_unit(self.reference_data, + limits_new = converter.to_unit(data, layer_state.attribute, limits_native, new_unit) From bccadee9cd26e528ac00ae24cf17abad1aa35fca Mon Sep 17 00:00:00 2001 From: Thomas Robitaille Date: Fri, 26 Apr 2024 12:31:08 +0100 Subject: [PATCH 2/3] Add profile viewer units test --- glue/core/application_base.py | 5 +- glue/viewers/profile/tests/test_viewer.py | 140 ++++++++++++++++++++++ 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/glue/core/application_base.py b/glue/core/application_base.py index 1f17683de..b200f87c0 100644 --- a/glue/core/application_base.py +++ b/glue/core/application_base.py @@ -54,6 +54,7 @@ def __init__(self, data_collection=None, session=None): self._hub = self._session.hub self._cmds = self._session.command_stack + self._cmds.add_callback(self._update_undo_redo_enabled) self._settings = {} @@ -275,8 +276,8 @@ def redo(self): except RuntimeError: pass - def _update_undo_redo_enabled(self): - raise NotImplementedError() + def _update_undo_redo_enabled(self, *args): + pass def add_datasets(self, *args, **kwargs): """ diff --git a/glue/viewers/profile/tests/test_viewer.py b/glue/viewers/profile/tests/test_viewer.py index 9c384ba1b..16a29b668 100644 --- a/glue/viewers/profile/tests/test_viewer.py +++ b/glue/viewers/profile/tests/test_viewer.py @@ -1,7 +1,16 @@ +from astropy import units as u +from astropy.wcs import WCS + +import numpy as np +from numpy.testing import assert_equal, assert_allclose + from glue.tests.visual.helpers import visual_test from glue.viewers.profile.viewer import SimpleProfileViewer from glue.core.application_base import Application from glue.core.data import Data +from glue.config import settings, unit_converter +from glue.plugins.wcs_autolinking.wcs_autolinking import WCSLink +from glue.core.roi import XRangeROI @visual_test @@ -25,3 +34,134 @@ def test_simple_viewer(): viewer.state.layers[2].linewidth = 5 return viewer.figure + + +@unit_converter('test-spectral2') +class SpectralUnitConverter: + + def equivalent_units(self, data, cid, units): + return map(str, u.Unit(units).find_equivalent_units(include_prefix_units=True, equivalencies=u.spectral())) + + def to_unit(self, data, cid, values, original_units, target_units): + return (values * u.Unit(original_units)).to_value(target_units, equivalencies=u.spectral()) + + +def test_unit_conversion(): + + settings.UNIT_CONVERTER = 'test-spectral2' + + wcs1 = WCS(naxis=1) + wcs1.wcs.ctype = ['FREQ'] + wcs1.wcs.crval = [1] + wcs1.wcs.cdelt = [1] + wcs1.wcs.crpix = [1] + wcs1.wcs.cunit = ['GHz'] + + d1 = Data(f1=[1, 2, 3]) + d1.get_component('f1').units = 'Jy' + d1.coords = wcs1 + + wcs2 = WCS(naxis=1) + wcs2.wcs.ctype = ['WAVE'] + wcs2.wcs.crval = [10] + wcs2.wcs.cdelt = [10] + wcs2.wcs.crpix = [1] + wcs2.wcs.cunit = ['cm'] + + d2 = Data(f2=[2000, 1000, 3000]) + d2.get_component('f2').units = 'mJy' + d2.coords = wcs2 + + app = Application() + session = app.session + + data_collection = session.data_collection + data_collection.append(d1) + data_collection.append(d2) + + data_collection.add_link(WCSLink(d1, d2)) + + viewer = app.new_data_viewer(SimpleProfileViewer) + viewer.add_data(d1) + viewer.add_data(d2) + + assert viewer.layers[0].enabled + assert viewer.layers[1].enabled + + x, y = viewer.state.layers[0].profile + assert_allclose(x, [1.e9, 2.e9, 3.e9]) + assert_allclose(y, [1, 2, 3]) + + x, y = viewer.state.layers[1].profile + assert_allclose(x, 299792458 / np.array([0.1, 0.2, 0.3])) + assert_allclose(y, [2000, 1000, 3000]) + + assert viewer.state.x_min == 1.e9 + assert viewer.state.x_max == 3.e9 + assert viewer.state.y_min == 1. + assert viewer.state.y_max == 3. + + # Change the limits to make sure they are always converted + viewer.state.x_min = 5e8 + viewer.state.x_max = 4e9 + viewer.state.y_min = 0.5 + viewer.state.y_max = 3.5 + + roi = XRangeROI(1.4e9, 2.1e9) + viewer.apply_roi(roi) + + assert len(d1.subsets) == 1 + assert_equal(d1.subsets[0].to_mask(), [0, 1, 0]) + + assert len(d2.subsets) == 1 + assert_equal(d2.subsets[0].to_mask(), [0, 1, 0]) + + viewer.state.x_display_unit = 'GHz' + viewer.state.y_display_unit = 'mJy' + + x, y = viewer.state.layers[0].profile + assert_allclose(x, [1, 2, 3]) + assert_allclose(y, [1000, 2000, 3000]) + + x, y = viewer.state.layers[1].profile + assert_allclose(x, 2.99792458 / np.array([1, 2, 3])) + assert_allclose(y, [2000, 1000, 3000]) + + assert viewer.state.x_min == 0.5 + assert viewer.state.x_max == 4. + + # Units get reset because they were originally 'native' and 'native' to a + # specific unit always trigger resetting the limits since different datasets + # might be converted in different ways. + assert viewer.state.y_min == 1000. + assert viewer.state.y_max == 3000. + + # Now set the limits explicitly again and make sure in future they are converted + viewer.state.y_min = 500. + viewer.state.y_max = 3500. + + roi = XRangeROI(0.5, 1.2) + viewer.apply_roi(roi) + + assert len(d1.subsets) == 1 + assert_equal(d1.subsets[0].to_mask(), [1, 0, 0]) + + assert len(d2.subsets) == 1 + assert_equal(d2.subsets[0].to_mask(), [0, 0, 1]) + + viewer.state.x_display_unit = 'cm' + viewer.state.y_display_unit = 'Jy' + + roi = XRangeROI(15, 35) + viewer.apply_roi(roi) + + assert len(d1.subsets) == 1 + assert_equal(d1.subsets[0].to_mask(), [1, 0, 0]) + + assert len(d2.subsets) == 1 + assert_equal(d2.subsets[0].to_mask(), [0, 1, 1]) + + assert_allclose(viewer.state.x_min, (4 * u.GHz).to_value(u.cm, equivalencies=u.spectral())) + assert_allclose(viewer.state.x_max, (0.5 * u.GHz).to_value(u.cm, equivalencies=u.spectral())) + assert_allclose(viewer.state.y_min, 0.5) + assert_allclose(viewer.state.y_max, 3.5) From 2b46109ece3419886a863b6463da2916a42da592 Mon Sep 17 00:00:00 2001 From: Thomas Robitaille Date: Fri, 26 Apr 2024 12:36:05 +0100 Subject: [PATCH 3/3] Added regression test --- glue/viewers/profile/tests/test_viewer.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/glue/viewers/profile/tests/test_viewer.py b/glue/viewers/profile/tests/test_viewer.py index 16a29b668..eb6f8e866 100644 --- a/glue/viewers/profile/tests/test_viewer.py +++ b/glue/viewers/profile/tests/test_viewer.py @@ -165,3 +165,9 @@ def test_unit_conversion(): assert_allclose(viewer.state.x_max, (0.5 * u.GHz).to_value(u.cm, equivalencies=u.spectral())) assert_allclose(viewer.state.y_min, 0.5) assert_allclose(viewer.state.y_max, 3.5) + + # Regression test for a bug that caused unit changes to not work on y axis + # if reference data was not first layer + + viewer.state.reference_data = d2 + viewer.state.y_display_unit = 'mJy'