WIP: improvements to left sidebar

glue/app/qt/preferences.py

@@ -28,7 +28,6 @@ class PreferencesDialog(QtWidgets.QDialog):
     data_alpha = ValueProperty('ui.slider_alpha', value_range=(0, 1))
     data_apply = ButtonProperty('ui.checkbox_apply')
     show_large_data_warning = ButtonProperty('ui.checkbox_show_large_data_warning')
-    individual_subset_color = ButtonProperty('ui.checkbox_individual_subset_color')
     save_to_disk = ButtonProperty('ui.checkbox_save')
 
     def __init__(self, application, parent=None):
@@ -58,7 +57,6 @@ def __init__(self, application, parent=None):
         self.data_color = settings.DATA_COLOR
         self.data_alpha = settings.DATA_ALPHA
         self.show_large_data_warning = settings.SHOW_LARGE_DATA_WARNING
-        self.individual_subset_color = settings.INDIVIDUAL_SUBSET_COLOR
 
         self._update_theme_from_colors()
 
@@ -104,7 +102,6 @@ def accept(self):
         settings.DATA_COLOR = self.data_color
         settings.DATA_ALPHA = self.data_alpha
         settings.SHOW_LARGE_DATA_WARNING = self.show_large_data_warning
-        settings.INDIVIDUAL_SUBSET_COLOR = self.individual_subset_color
 
         for pane in self.panes:
             pane.finalize()

glue/app/qt/preferences.ui

@@ -186,13 +186,6 @@
          </property>
         </widget>
        </item>
-       <item row="9" column="2">
-        <widget class="QCheckBox" name="checkbox_individual_subset_color">
-         <property name="text">
-          <string/>
-         </property>
-        </widget>
-       </item>
       </layout>
      </widget>
     </widget>

glue/config.py

@@ -771,4 +771,3 @@ def _default_search_order():
 settings.add('BACKGROUND_COLOR', '#FFFFFF')
 settings.add('FOREGROUND_COLOR', '#000000')
 settings.add('SHOW_LARGE_DATA_WARNING', True, validator=bool)
-settings.add('INDIVIDUAL_SUBSET_COLOR', False, validator=bool)

glue/core/application_base.py

@@ -6,12 +6,12 @@
 from glue.core.session import Session
 from glue.core.edit_subset_mode import EditSubsetMode
 from glue.core.hub import HubListener
-from glue.core import Data, Subset
+from glue.core import Data, SubsetGroup
 from glue.core import command
 from glue.core.data_factories import load_data
 from glue.core.data_collection import DataCollection
 from glue.config import settings
-from glue.utils import as_list, PropertySetMixin
+from glue.utils import PropertySetMixin
 
 
 __all__ = ['Application', 'ViewerBase']
@@ -362,9 +362,9 @@ def request_add_layer(self, layer):
     def add_layer(self, layer):
         if isinstance(layer, Data):
             self.add_data(layer)
-        elif isinstance(layer, Subset):
-            self.add_subset(layer)
-        # else: SubsetGroup
+        elif isinstance(layer, SubsetGroup):
+            for subset in layer.subsets:
+                self.add_subset(subset)
 
     def add_data(self, data):
         """ Add a data instance to the viewer

glue/core/data.py

@@ -656,6 +656,7 @@ def add_subset(self, subset):
             Manually-instantiated subsets will **not** be
             represented properly by the UI
         """
+
         if subset in self.subsets:
             return  # prevents infinite recursion
         if isinstance(subset, SubsetState):

glue/core/qt/data_collection_model.py

@@ -155,12 +155,13 @@ def font(self):
 
 class SubsetGroupItem(Item):
     edit_factory = full_edit_factory
-    flags = Qt.ItemIsSelectable | Qt.ItemIsEnabled | Qt.ItemIsEditable
+    flags = Qt.ItemIsSelectable | Qt.ItemIsEnabled | Qt.ItemIsEditable | Qt.ItemIsDragEnabled
 
     def __init__(self, dc, row, parent):
         self.parent = parent
         self.dc = dc
         self.row = row
+        self.children_count = 0
         self.column = 0
 
     @property
@@ -195,50 +196,10 @@ def tooltip(self):
     def style(self):
         return self.subset_group.style
 
-    @property
-    def children_count(self):
-        return len(self.subset_group.subsets)
-
-    @memoize
-    def child(self, row):
-        return SubsetItem(self.dc, self.subset_group, row, self)
-
     def icon(self):
         return layer_icon(self.subset_group)
 
 
-class SubsetItem(Item):
-    edit_factory = restricted_edit_factory
-    flags = Qt.ItemIsSelectable | Qt.ItemIsEnabled | Qt.ItemIsDragEnabled
-
-    def __init__(self, dc, subset_group, subset_idx, parent):
-        self.parent = parent
-        self.subset_group = subset_group
-        self.row = subset_idx
-        self.parent = parent
-        self.children_count = 0
-        self.column = 0
-
-    @property
-    def subset(self):
-        return self.subset_group.subsets[self.row]
-
-    @property
-    def label(self):
-        return self.subset.verbose_label
-
-    def icon(self):
-        return layer_icon(self.subset)
-
-    @property
-    def style(self):
-        return self.subset.style
-
-    @property
-    def glue_data(self):
-        return self.subset
-
-
 class DataCollectionModel(QtCore.QAbstractItemModel, HubListener):
     new_item = QtCore.Signal(QtCore.QModelIndex)
 
@@ -262,16 +223,19 @@ def index(self, row, column, parent=QtCore.QModelIndex()):
         if column != 0:
             return QtCore.QModelIndex()
 
-        if not parent.isValid():
-            parent_item = self.root
-        else:
+        if parent.isValid():
             parent_item = self._get_item(parent)
             if parent_item is None:
                 return QtCore.QModelIndex()
+        else:
+            parent_item = self.root
 
-        child_item = parent_item.child(row)
-        if child_item:
-            return self._make_index(row, column, child_item)
+        if parent_item.children_count > 0:
+            child_item = parent_item.child(row)
+            if child_item:
+                return self._make_index(row, column, child_item)
+            else:
+                return QtCore.QModelIndex()
         else:
             return QtCore.QModelIndex()
 
@@ -385,6 +349,7 @@ def subsets_index(self, subset_number=None):
         return self.index(subset_number, 0, base)
 
     def rowCount(self, index=QtCore.QModelIndex()):
+
         item = self._get_item(index)
 
         if item is None:

glue/core/qt/tests/test_data_collection_model.py

@@ -35,17 +35,17 @@ def test_row_count_subset_row(self):
 
     def test_row_count_single_subset1(self):
         model = self.make_model(2, 1)
-        assert model.rowCount(model.subsets_index(0)) == 2
+        assert model.rowCount(model.subsets_index(0)) == 0
 
     def test_row_count_single_subset2(self):
         model = self.make_model(2, 1)
         s = model.subsets_index(0)
 
         idx = model.index(0, 0, s)
-        assert model.rowCount(idx) == 0
+        assert model.rowCount(idx) == 2
 
         idx = model.index(1, 0, s)
-        assert model.rowCount(s) == 2
+        assert model.rowCount(s) == 0
 
     def test_invalid_indices(self):
         model = self.make_model(1, 2)
@@ -74,8 +74,6 @@ def test_dc_labels(self):
         assert model.data(model.data_index(0), Qt.DisplayRole) == 'test1'
         assert model.data(model.subsets_index(0), Qt.DisplayRole) == 'subset1'
         assert model.data(model.subsets_index(1), Qt.DisplayRole) == 'subset2'
-        assert model.data(model.index(0, 0, model.subsets_index(0)),
-                          Qt.DisplayRole) == 'subset1 (test1)'
 
     def test_column_count(self):
         model = self.make_model(1, 2)
@@ -101,7 +99,7 @@ def test_font_role(self):
     def test_drag_flags(self):
         model = self.make_model(1, 2)
 
-        sg = model.subsets_index(0)
+        sg = model.subsets_index()
         subset = model.index(0, 0, sg)
         assert model.flags(model.data_index(0)) & Qt.ItemIsDragEnabled
         assert model.flags(subset) & Qt.ItemIsDragEnabled
@@ -126,9 +124,8 @@ def test_layers_mime_type_data(self):
     def test_layers_mime_type_multiselection(self):
         model = self.make_model(1, 2)
         idxs = [model.data_index(0),
-                model.subsets_index(0),
-                model.index(0, 0, model.subsets_index(0))]
+                model.subsets_index(0)]
 
         dc = model.data_collection
-        expected = [dc[0], dc.subset_groups[0], dc.subset_groups[0].subsets[0]]
+        expected = [dc[0], dc.subset_groups[0]]
         assert model.mimeData(idxs).data(LAYERS_MIME_TYPE) == expected

glue/core/subset.py

@@ -61,22 +61,18 @@ def __init__(self, data, color=settings.SUBSET_COLORS[0], alpha=0.5, label=None)
         via DataCollection.new_subset_group. Manually-instantiated
         subsets will probably *not* be represented properly by the UI
         """
+
         self._broadcasting = False  # must be first def
+
         self.data = data
-        self._subset_state = None
-        self._label = None
-        self._style = None
-        self._setup(color, alpha, label)
-
-    @contract(color='color', alpha='float', label='string|None')
-    def _setup(self, color, alpha, label):
-        self.color = color
         self.label = label  # trigger disambiguation
+
+        self.subset_state = SubsetState()  # calls proper setter method
+
         self.style = VisualAttributes(parent=self)
         self.style.markersize *= 1.5
         self.style.color = color
         self.style.alpha = alpha
-        self.subset_state = SubsetState()  # calls proper setter method
 
     @property
     def subset_state(self):
@@ -289,6 +285,7 @@ def broadcast(self, attribute):
         :type attribute: ``str``
 
         """
+
         if not hasattr(self, 'data') or not hasattr(self.data, 'hub'):
             return
 
@@ -402,6 +399,8 @@ def __eq__(self, other):
         if not isinstance(other, Subset):
             return False
         # XXX need to add equality specification for subset states
+        if self is other:
+            return True
         return (self.subset_state == other.subset_state and
                 self.style == other.style)
 

glue/core/subset_group.py

@@ -19,8 +19,7 @@
 
 from glue.external import six
 from glue.core.contracts import contract
-from glue.core.message import (SubsetUpdateMessage,
-                               DataCollectionAddMessage,
+from glue.core.message import (DataCollectionAddMessage,
                                DataCollectionDeleteMessage)
 from glue.core.visual import VisualAttributes
 from glue.core.hub import HubListener
@@ -47,27 +46,30 @@ def __init__(self, data, group):
         :param data: :class:`~glue.core.data.Data` instance to bind to
         :param group: :class:`~glue.core.subset_group.SubsetGroup`
         """
+
+        # We deliberately don't call Subset.__init__ here because we don't want
+        # to set e.g. the subset state, color, transparency, etc. Instead we
+        # just want to defer to the SubsetGroup for these.
+
+        self._broadcasting = False  # must be first def
+
         self.group = group
-        super(GroupedSubset, self).__init__(data, label=group.label,
-                                            color=group.style.color,
-                                            alpha=group.style.alpha)
 
-    def _setup(self, color, alpha, label):
-        self.color = color
-        self.label = label  # trigger disambiguation
-        self.style = VisualAttributes(parent=self)
-        self.style.markersize *= 2.5
-        self.style.color = color
-        self.style.alpha = alpha
-        # skip state setting here
+        self.data = data
+        self.label = group.label  # trigger disambiguation
+
+    @property
+    def style(self):
+        return self.group.style
+
+    @property
+    def subset_group(self):
+        return self.group.subset_group
 
     @property
     def verbose_label(self):
         return "%s (%s)" % (self.label, self.data.label)
 
-    def sync_style(self, other):
-        self.style.set(other)
-
     def __eq__(self, other):
         return other is self
 
@@ -85,7 +87,6 @@ def __setgluestate__(cls, rec, context):
         self = cls(None, dummy_grp)
         yield self
         self.group = context.object(rec['group'])
-        self.style = context.object(rec['style'])
 
 
 class SubsetGroup(HubListener):
@@ -98,12 +99,13 @@ def __init__(self, color=settings.SUBSET_COLORS[0], alpha=0.5, label=None, subse
         DataCollection.new_subset.
         """
         self.subsets = []
+
         if subset_state is None:
-            subset_state = SubsetState()
+            self.subset_state = SubsetState()
+        else:
+            self.subset_state = subset_state
 
-        self.subset_state = subset_state
         self.label = label
-        self._style = None
 
         self.style = VisualAttributes(parent=self)
         self.style.markersize *= 2.5
@@ -155,39 +157,16 @@ def register_to_hub(self, hub):
         hub.subscribe(self, DataCollectionDeleteMessage,
                       lambda x: self._remove_data(x.data))
 
-        def has_subset(msg):
-            return msg.attribute == 'style' and msg.subset in self.subsets
-
-        hub.subscribe(self, SubsetUpdateMessage,
-                      self._sync_style_from_subset,
-                      filter=has_subset)
-
-    def _sync_style_from_subset(self, msg):
-        if settings.INDIVIDUAL_SUBSET_COLOR:
-            return
-        self._style.set(msg.subset.style)
-        self.broadcast('style')
-
     @property
     def style(self):
         return self._style
 
     @style.setter
     def style(self, value):
         self._style = value
-        self._sync_style()
-
-    def _sync_style(self):
-        for s in self.subsets:
-            s.sync_style(self.style)
 
     @contract(item='string')
     def broadcast(self, item):
-        # used by __setattr__ and VisualAttributes.__setattr__
-        if item == 'style':
-            self._sync_style()
-            return
-
         for s in self.subsets:
             s.broadcast(item)