Implement a new way to deal with combo boxes where the choices are de…

…fined as part of the State, not in the Qt code

glue/core/qt/data_combo_helper.py

@@ -17,6 +17,52 @@
            'DataCollectionComboHelper']
 
 
+class QtComboHelper(object):
+
+    def __init__(self, combo, state, selection_property, choices_property):
+        self.combo = combo
+        self.state = state
+        self.selection_property = selection_property
+        self.choices_property = choices_property
+        self.state.add_callback(selection_property, self._selection_changed)
+        self.state.add_callback(selection_property, self._choices_changed)
+
+    def _choices_changed(self, *args):
+
+        choices = getattr(self.state, self.choices_property)
+
+        self.combo.blockSignals(True)
+
+        self.combo.clear()
+
+        if len(choices) == 0:
+            return
+
+        combo_model = self.combo.model()
+
+        for index, (label, data) in enumerate(choices):
+
+            self.combo.addItem(label, userData=data)
+
+            # We interpret None data as being disabled rows (used for headers)
+            if data is None:
+                item = combo_model.item(index)
+                palette = self.combo.palette()
+                item.setFlags(item.flags() & ~(Qt.ItemIsSelectable | Qt.ItemIsEnabled))
+                item.setData(palette.color(QtGui.QPalette.Disabled, QtGui.QPalette.Text))
+
+        self._selection_changed()
+
+        self.combo.blockSignals(False)
+
+        self.combo.currentIndexChanged.emit(index)
+
+    def _selection_changed(self, *args):
+        index = self.combo.findData(getattr(self.state, self.selection_property))
+        if self.combo.currentIndex() != index:
+            self.combo.setCurrentIndex(index)
+
+
 class ComponentIDComboHelper(HubListener):
     """
     The purpose of this class is to set up a combo showing componentIDs for
@@ -138,9 +184,7 @@ def append_data(self, data, refresh=True):
             data = data.data
 
         if self.hub is None:
-            if data.hub is None:
-                raise ValueError("Hub is not set on Data object")
-            else:
+            if data.hub is not None:
                 self.hub = data.hub
         elif data.hub is not self.hub:
             raise ValueError("Data Hub is different from current hub")

glue/core/state_objects.py

@@ -76,7 +76,7 @@ def as_dict(self):
         """
         properties = {}
         for name in dir(self):
-            if self.is_callback_property(name):
+            if self.is_callback_property(name) and not name.startswith('_'):
                 properties[name] = getattr(self, name)
         return properties
 

glue/viewers/image/qt/options_widget.py

@@ -4,11 +4,9 @@
 
 from qtpy import QtWidgets
 
-from glue.core.data import Data
-from glue.external.echo import delay_callback
 from glue.external.echo.qt import autoconnect_callbacks_to_qt
 from glue.utils.qt import load_ui
-from glue.core.qt.data_combo_helper import ComponentIDComboHelper, ManualDataComboHelper
+from glue.core.qt.data_combo_helper import QtComboHelper
 from glue.viewers.image.qt.slice_widget import MultiSliceWidgetHelper
 
 __all__ = ['ImageOptionsWidget']
@@ -23,9 +21,6 @@ def __init__(self, viewer_state, session, parent=None):
         self.ui = load_ui('options_widget.ui', self,
                           directory=os.path.dirname(__file__))
 
-        viewer_state.add_callback('layers', self._update_combo_ref_data, priority=1500)
-        viewer_state.add_callback('reference_data', self._update_combo_att)
-
         self.ui.combodata_aspect.addItem("Square Pixels", userData='equal')
         self.ui.combodata_aspect.addItem("Automatic", userData='auto')
         self.ui.combodata_aspect.setCurrentIndex(0)
@@ -35,38 +30,17 @@ def __init__(self, viewer_state, session, parent=None):
 
         autoconnect_callbacks_to_qt(viewer_state, self.ui)
 
-        self.ref_data_helper = ManualDataComboHelper(self.ui.combodata_reference_data,
-                                                     session.data_collection)
-
-        self.x_att_helper = ComponentIDComboHelper(self.ui.combodata_x_att_world,
-                                                   session.data_collection,
-                                                   numeric=False, categorical=False,
-                                                   visible=False, world_coord=True,
-                                                   default_index=-1)
-
-        self.y_att_helper = ComponentIDComboHelper(self.ui.combodata_y_att_world,
-                                                   session.data_collection,
-                                                   numeric=False, categorical=False,
-                                                   visible=False, world_coord=True,
-                                                   default_index=-2)
+        self.ref_data_helper = QtComboHelper(self.ui.combonew_reference_data,
+                                             viewer_state, 'reference_data',
+                                             '_reference_data_choices')
+        self.x_att_world_helper = QtComboHelper(self.ui.combonew_x_att_world,
+                                                viewer_state, 'x_att_world',
+                                                '_x_att_world_choices')
+        self.y_att_world_helper = QtComboHelper(self.ui.combonew_y_att_world,
+                                                viewer_state, 'y_att_world',
+                                                '_y_att_world_choices')
 
         self.viewer_state = viewer_state
 
         self.slice_helper = MultiSliceWidgetHelper(viewer_state=self.viewer_state,
                                                    widget=self.ui.slice_tab)
-
-    def _update_combo_ref_data(self, *args):
-        datasets = []
-        for layer in self.viewer_state.layers:
-            if isinstance(layer.layer, Data):
-                if layer.layer not in datasets:
-                    datasets.append(layer.layer)
-            else:
-                if layer.layer.data not in datasets:
-                    datasets.append(layer.layer.data)
-        self.ref_data_helper.set_multiple_data(datasets)
-
-    def _update_combo_att(self, *args):
-        with delay_callback(self.viewer_state, 'x_att_world', 'y_att_world'):
-            self.x_att_helper.set_multiple_data([self.viewer_state.reference_data])
-            self.y_att_helper.set_multiple_data([self.viewer_state.reference_data])

glue/viewers/image/qt/options_widget.ui

@@ -21,7 +21,7 @@
     <number>5</number>
    </property>
    <item row="1" column="0">
-    <widget class="QComboBox" name="combodata_reference_data"/>
+    <widget class="QComboBox" name="combonew_reference_data"/>
    </item>
    <item row="0" column="0">
     <widget class="QLabel" name="label_4">
@@ -118,14 +118,14 @@
         <widget class="QComboBox" name="combodata_aspect"/>
        </item>
        <item row="0" column="2" colspan="3">
-        <widget class="QComboBox" name="combodata_x_att_world">
+        <widget class="QComboBox" name="combonew_x_att_world">
          <property name="sizeAdjustPolicy">
           <enum>QComboBox::AdjustToMinimumContentsLength</enum>
          </property>
         </widget>
        </item>
        <item row="2" column="2" colspan="3">
-        <widget class="QComboBox" name="combodata_y_att_world">
+        <widget class="QComboBox" name="combonew_y_att_world">
          <property name="sizeAdjustPolicy">
           <enum>QComboBox::AdjustToMinimumContentsLength</enum>
          </property>

glue/viewers/image/qt/tests/test_viewer_widget.py

@@ -104,8 +104,8 @@ def test_basic(self):
 
         self.viewer.add_data(self.image1)
 
-        assert combo_as_string(self.options_widget.ui.combodata_x_att_world) == 'World 0:World 1'
-        assert combo_as_string(self.options_widget.ui.combodata_x_att_world) == 'World 0:World 1'
+        assert combo_as_string(self.options_widget.ui.combonew_x_att_world) == 'World 0:World 1'
+        assert combo_as_string(self.options_widget.ui.combonew_x_att_world) == 'World 0:World 1'
 
         assert self.viewer.axes.get_xlabel() == 'World 1'
         assert self.viewer.state.x_att_world is self.image1.id['World 1']
@@ -132,8 +132,8 @@ def test_custom_coords(self):
 
         self.viewer.add_data(self.image2)
 
-        assert combo_as_string(self.options_widget.ui.combodata_x_att_world) == 'Banana:Apple'
-        assert combo_as_string(self.options_widget.ui.combodata_x_att_world) == 'Banana:Apple'
+        assert combo_as_string(self.options_widget.ui.combonew_x_att_world) == 'Banana:Apple'
+        assert combo_as_string(self.options_widget.ui.combonew_x_att_world) == 'Banana:Apple'
 
         assert self.viewer.axes.get_xlabel() == 'Apple'
         assert self.viewer.state.x_att_world is self.image2.id['Apple']
@@ -239,8 +239,8 @@ def test_hypercube(self):
 
         self.viewer.add_data(self.hypercube)
 
-        assert combo_as_string(self.options_widget.ui.combodata_x_att_world) == 'World 0:World 1:World 2:World 3'
-        assert combo_as_string(self.options_widget.ui.combodata_x_att_world) == 'World 0:World 1:World 2:World 3'
+        assert combo_as_string(self.options_widget.ui.combonew_x_att_world) == 'World 0:World 1:World 2:World 3'
+        assert combo_as_string(self.options_widget.ui.combonew_x_att_world) == 'World 0:World 1:World 2:World 3'
 
         assert self.viewer.axes.get_xlabel() == 'World 3'
         assert self.viewer.state.x_att_world is self.hypercube.id['World 3']

glue/viewers/image/state.py

@@ -8,6 +8,7 @@
 from glue.core.state_objects import StateAttributeLimitsHelper
 from glue.utils import defer_draw
 from glue.external.echo import delay_callback
+from glue.core.data_combo_helper import ManualDataComboHelper, ComponentIDComboHelper
 
 __all__ = ['ImageViewerState', 'ImageLayerState', 'ImageSubsetLayerState']
 
@@ -17,6 +18,10 @@ class ImageViewerState(MatplotlibDataViewerState):
     A state class that includes all the attributes for an image viewer.
     """
 
+    _x_att_world_choices = DDCProperty()
+    _y_att_world_choices = DDCProperty()
+    _reference_data_choices = DDCProperty()
+
     x_att = DDCProperty(docstring='The component ID giving the pixel component '
                                   'shown on the x axis')
     y_att = DDCProperty(docstring='The component ID giving the pixel component '
@@ -51,8 +56,23 @@ def __init__(self, **kwargs):
                                                        lower='y_min', upper='y_max',
                                                        limits_cache=self.limits_cache)
 
-        self.add_callback('reference_data', self._set_default_slices)
-        self.add_callback('layers', self._set_reference_data)
+        self.ref_data_helper = ManualDataComboHelper(self, 'reference_data',
+                                                     '_reference_data_choices')
+
+        self.xw_att_helper = ComponentIDComboHelper(self, 'x_att_world',
+                                                    '_x_att_world_choices',
+                                                    numeric=False, categorical=False,
+                                                    visible=False, world_coord=True,
+                                                    default_index=-1)
+
+        self.yw_att_helper = ComponentIDComboHelper(self, 'y_att_world',
+                                                    '_y_att_world_choices',
+                                                    numeric=False, categorical=False,
+                                                    visible=False, world_coord=True,
+                                                    default_index=-2)
+
+        self.add_callback('reference_data', self._reference_data_changed)
+        self.add_callback('layers', self._layers_changed)
 
         self.add_callback('x_att', self._on_xatt_change, priority=500)
         self.add_callback('y_att', self._on_yatt_change, priority=500)
@@ -63,6 +83,36 @@ def __init__(self, **kwargs):
         self.add_callback('x_att_world', self._on_xatt_world_change, priority=1000)
         self.add_callback('y_att_world', self._on_yatt_world_change, priority=1000)
 
+    def _reference_data_changed(self, *args):
+        with delay_callback(self, 'x_att_world', 'y_att_world', 'slices'):
+            self._update_combo_att()
+            self._set_default_slices()
+
+    def _layers_changed(self, *args):
+        self._update_combo_ref_data()
+        self._set_reference_data()
+
+    def _update_combo_ref_data(self, *args):
+        datasets = []
+        for layer in self.layers:
+            if isinstance(layer.layer, Data):
+                if layer.layer not in datasets:
+                    datasets.append(layer.layer)
+            else:
+                if layer.layer.data not in datasets:
+                    datasets.append(layer.layer.data)
+        self.ref_data_helper.set_multiple_data(datasets)
+
+    def _update_combo_att(self, *args):
+        with delay_callback(self, 'x_att_world', 'y_att_world'):
+            if self.reference_data is None:
+                self.xw_att_helper.set_multiple_data([])
+                self.yw_att_helper.set_multiple_data([])
+            else:
+                self.xw_att_helper.set_multiple_data([self.reference_data])
+                self.yw_att_helper.set_multiple_data([self.reference_data])
+
+
     def _update_priority(self, name):
         if name == 'layers':
             return 3
@@ -98,7 +148,7 @@ def _on_yatt_change(self, *args):
 
     @defer_draw
     def _on_xatt_world_change(self, *args):
-        if self.x_att_world == self.y_att_world:
+        if self.x_att_world is not None and self.x_att_world == self.y_att_world:
             world_ids = self.reference_data.world_component_ids
             if self.x_att_world == world_ids[-1]:
                 self.y_att_world = world_ids[-2]
@@ -107,7 +157,7 @@ def _on_xatt_world_change(self, *args):
 
     @defer_draw
     def _on_yatt_world_change(self, *args):
-        if self.y_att_world == self.x_att_world:
+        if self.y_att_world is not None and self.y_att_world == self.x_att_world:
             world_ids = self.reference_data.world_component_ids
             if self.y_att_world == world_ids[-1]:
                 self.x_att_world = world_ids[-2]