Merge pull request #2486 from astrofrog/fix-profile-ylimits-units

Fix bug that caused IncompatibleAttribute error when trying to change limits

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):
         """

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)
 

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,140 @@ 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)
+
+    # 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'