Merge pull request #2394 from astrofrog/unit-limits-update

Avoid resetting limits when changing display units in profile viewer

glue/viewers/profile/qt/tests/test_data_viewer.py

@@ -403,6 +403,12 @@ def test_unit_conversion():
     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)
 
@@ -423,11 +429,19 @@ def test_unit_conversion():
     assert_allclose(x, 2.99792458 / np.array([1, 2, 3]))
     assert_allclose(y, [2000, 1000, 3000])
 
-    assert viewer.state.x_min == 1.
-    assert viewer.state.x_max == 3.
+    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)
 
@@ -438,6 +452,7 @@ def test_unit_conversion():
     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)
@@ -447,3 +462,8 @@ def test_unit_conversion():
 
     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)

glue/viewers/profile/state.py

@@ -62,8 +62,8 @@ def __init__(self, **kwargs):
         self.add_callback('layers', self._layers_changed)
         self.add_callback('reference_data', self._reference_data_changed, echo_old=True)
         self.add_callback('x_att', self._update_att)
-        self.add_callback('x_display_unit', self._reset_x_limits)
-        self.add_callback('y_display_unit', self._reset_y_limits_if_changed, echo_old=True)
+        self.add_callback('x_display_unit', self._convert_units_x_limits, echo_old=True)
+        self.add_callback('y_display_unit', self._convert_units_y_limits, echo_old=True)
         self.add_callback('normalize', self._reset_y_limits)
         self.add_callback('function', self._reset_y_limits)
 
@@ -85,11 +85,64 @@ def format_unit(unit):
 
         self.update_from_dict(kwargs)
 
-    def _reset_y_limits_if_changed(self, old, new):
-        # This is needed because otherwise reset_y_limits gets called even if
-        # just the choices in the y units change but not the selected value.
-        if old != new:
-            self._reset_y_limits()
+    def _convert_units_x_limits(self, old_unit, new_unit):
+
+        if old_unit != new_unit and self.reference_data is not None:
+
+            limits = np.array([self.x_min, self.x_max])
+
+            converter = UnitConverter()
+
+            limits_native = converter.to_native(self.reference_data,
+                                                self.x_att, limits,
+                                                old_unit)
+
+            limits_new = converter.to_unit(self.reference_data,
+                                           self.x_att, limits_native,
+                                           new_unit)
+
+            with delay_callback(self, 'x_min', 'x_max'):
+                self.x_min, self.x_max = sorted(limits_new)
+
+    def _convert_units_y_limits(self, old_unit, new_unit):
+
+        if old_unit != new_unit:
+
+            if old_unit is None or new_unit is None:
+                self._reset_y_limits()
+                return
+
+            limits = np.array([self.y_min, self.y_max])
+
+            converter = UnitConverter()
+
+            # We can use any layer, just find the first one that works and
+            # exit if none of them do.
+            for layer_state in self.layers:
+                try:
+                    layer_state.profile
+                except Exception:  # e.g. incompatible subset
+                    continue
+                else:
+                    break
+            else:
+                return
+
+            if isinstance(layer_state.layer, Subset):
+                data = layer_state.layer.data
+            else:
+                data = layer_state.layer
+
+            limits_native = converter.to_native(data,
+                                                layer_state.attribute, limits,
+                                                old_unit)
+
+            limits_new = converter.to_unit(self.reference_data,
+                                           layer_state.attribute, limits_native,
+                                           new_unit)
+
+            with delay_callback(self, 'y_min', 'y_max'):
+                self.y_min, self.y_max = sorted(limits_new)
 
     def _update_combo_ref_data(self):
         self.ref_data_helper.set_multiple_data(self.layers_data)