Merge pull request #1367 from astrofrog/layer-visibility

Various fixes, including disabling layers properly in UI

CHANGES.md

@@ -4,6 +4,15 @@ Full changelog
 v0.11.0 (unreleased)
 --------------------
 
+* Disabled layer artists can no longer be selected to avoid any confusion. [#1367]
+
+* Layer artist icons can now show colormaps when appropriate. [#1367]
+
+* Fix behavior of data wizard so that it doesn't overwrite labels set by data
+  factories. [#1367]
+
+* Add a status tip for all ROI selection tools. [#1367]
+
 * Fixed a bug that caused the terminal to not be available after
   resetting or opening a session. [#1366]
 

glue/core/data_combo_helper.py

@@ -5,7 +5,7 @@
 
 import weakref
 
-from glue.core import Subset
+from glue.core import Data, Subset
 from glue.core.hub import HubListener
 from glue.core.message import (ComponentsChangedMessage,
                                DataCollectionAddMessage,
@@ -19,6 +19,22 @@
            'DataCollectionComboHelper']
 
 
+def unique_data_iter(datasets):
+    """
+    Return a list with only Data objects, with duplicates removed, but
+    preserving the original order.
+    """
+    datasets_new = []
+    for dataset in datasets:
+        if isinstance(dataset, Data):
+            if dataset not in datasets_new:
+                datasets_new.append(dataset)
+        else:
+            if dataset.data not in datasets_new:
+                datasets_new.append(dataset.data)
+    return datasets_new
+
+
 class ComboHelper(HubListener):
     """
     Base class for any combo helper represented by a SelectionCallbackProperty.
@@ -250,7 +266,7 @@ def set_multiple_data(self, datasets):
             self._data.clear()
         except AttributeError:  # PY2
             self._data[:] = []
-        for data in datasets:
+        for data in unique_data_iter(datasets):
             self.append_data(data, refresh=False)
         self.refresh()
 
@@ -423,7 +439,7 @@ def set_multiple_data(self, datasets):
             self._datasets.clear()
         except AttributeError:  # PY2
             self._datasets[:] = []
-        for data in datasets:
+        for data in unique_data_iter(datasets):
             self._datasets.append(data)
         self.refresh()
 

glue/core/layer_artist.py

@@ -20,7 +20,7 @@
 from glue.external import six
 from glue.core.subset import Subset
 from glue.utils import Pointer, PropertySetMixin
-
+from glue.core.message import LayerArtistEnabledMessage, LayerArtistDisabledMessage
 
 __all__ = ['LayerArtistBase', 'MatplotlibLayerArtist', 'LayerArtistContainer']
 
@@ -83,12 +83,21 @@ def __init__(self, layer):
 
         self._disabled_reason = ''  # A string explaining why this layer is disabled.
 
+    def get_layer_color(self):
+        # This method can return either a plain color or a colormap. This is
+        # used by the UI layer to determine a 'representative' color or colormap
+        # for the layer to be used e.g. in icons.
+        return self._layer.style.color
+
     def enable(self):
         if self.enabled:
             return
         self._disabled_reason = ''
         self._enabled = True
         self.redraw()
+        if self._layer is not None and self._layer.hub is not None:
+            message = LayerArtistEnabledMessage(self)
+            self._layer.hub.broadcast(message)
 
     def disable(self, reason):
         """
@@ -106,6 +115,9 @@ def disable(self, reason):
         self._disabled_reason = reason
         self._enabled = False
         self.clear()
+        if self._layer is not None and self._layer.hub is not None:
+            message = LayerArtistDisabledMessage(self)
+            self._layer.hub.broadcast(message)
 
     def disable_invalid_attributes(self, *attributes):
         """

glue/core/message.py

@@ -11,7 +11,8 @@
            'DataCollectionMessage', 'DataCollectionActiveChange',
            'DataCollectionActiveDataChange', 'DataCollectionAddMessage',
            'DataCollectionDeleteMessage', 'ApplicationClosedMessage',
-           'DataRemoveComponentMessage']
+           'DataRemoveComponentMessage', 'LayerArtistEnabledMessage',
+           'LayerArtistDisabledMessage']
 
 
 class Message(object):
@@ -204,3 +205,15 @@ def __init__(self, sender, settings, tag=None):
 class ApplicationClosedMessage(Message):
     """A general message issued when Glue application is closed."""
     pass
+
+
+class LayerArtistEnabledMessage(Message):
+    def __init__(self, sender, tag=None):
+        super(LayerArtistEnabledMessage, self).__init__(sender, tag=tag)
+        self.layer_artist = self.sender
+
+
+class LayerArtistDisabledMessage(Message):
+    def __init__(self, sender, tag=None):
+        super(LayerArtistDisabledMessage, self).__init__(sender, tag=tag)
+        self.layer_artist = self.sender

glue/core/qt/layer_artist_model.py

@@ -22,7 +22,7 @@
 from glue.utils import nonpartial
 from glue.utils.qt import PythonListModel, PyMimeData
 from glue.core.hub import HubListener
-from glue.core.message import Message
+from glue.core.message import Message, LayerArtistEnabledMessage, LayerArtistDisabledMessage
 
 
 class LayerArtistModel(PythonListModel):
@@ -62,10 +62,16 @@ def data(self, index, role):
     def flags(self, index):
         result = super(LayerArtistModel, self).flags(index)
         if index.isValid():
-            result = (result | Qt.ItemIsEditable | Qt.ItemIsDragEnabled |
-                      Qt.ItemIsUserCheckable)
+            art = self.artists[index.row()]
+            if art.enabled:
+                result = (result | Qt.ItemIsEditable | Qt.ItemIsDragEnabled |
+                          Qt.ItemIsUserCheckable)
+            else:
+                result = (result & Qt.ItemIsEnabled) ^ result
+                result = (result & Qt.ItemIsSelectable) ^ result
+                result = (result & Qt.ItemIsUserCheckable) ^ result
         else:  # only drop between rows, where index isn't valid
-            result = (result | Qt.ItemIsDropEnabled)
+            result = result | Qt.ItemIsDropEnabled
 
         return result
 
@@ -197,12 +203,30 @@ def __init__(self, parent=None, hub=None):
         # listen to all events since the viewport update is fast.
         self.hub = hub
         self.hub.subscribe(self, Message, self._update_viewport)
+        self.hub.subscribe(self, LayerArtistEnabledMessage, self._layer_enabled_or_disabled)
+        self.hub.subscribe(self, LayerArtistDisabledMessage, self._layer_enabled_or_disabled)
 
     def _update_viewport(self, *args):
+
         # This forces the widget containing the list view to update/redraw,
         # reflecting any changes in color/labels/content
         self.viewport().update()
 
+    def _layer_enabled_or_disabled(self, *args):
+
+        # This forces the widget containing the list view to update/redraw,
+        # reflecting any changes in disabled/enabled layers. If a layer is
+        # disabled, it will become unselected.
+        self.viewport().update()
+
+        # If a layer artist becomes unselected as a result of the update above,
+        # a selection change event is not emitted for some reason, so we force
+        # a manual update. If a layer artist was deselected, current_artist()
+        # will be None and the options will be hidden for that layer.
+        parent = self.parent()
+        if parent is not None:
+            parent.on_selection_change(self.current_artist())
+
     def rowsInserted(self, index, start, end):
         super(LayerArtistView, self).rowsInserted(index, start, end)
         # If no rows are currently selected, make sure we select one. We do
@@ -216,7 +240,6 @@ def rowsInserted(self, index, start, end):
 
     def selectionChanged(self, selected, deselected):
         super(LayerArtistView, self).selectionChanged(selected, deselected)
-        self._update_actions()
         parent = self.parent()
         if parent is not None:
             parent.on_selection_change(self.current_artist())
@@ -242,9 +265,6 @@ def current_row(self):
             return
         return rows[0].row()
 
-    def _update_actions(self):
-        pass
-
     def _bottom_left_of_current_index(self):
         idx = self.currentIndex()
         if not idx.isValid():

glue/core/qt/tests/test_layer_artist_model.py

@@ -5,7 +5,7 @@
 from qtpy.QtCore import Qt
 from qtpy import PYQT5
 from glue.core import Data, Hub
-from glue.core.layer_artist import MatplotlibLayerArtist as _LayerArtist
+from glue.core.layer_artist import LayerArtistBase as _LayerArtist
 
 from ..layer_artist_model import LayerArtistModel, LayerArtistView
 
@@ -15,10 +15,15 @@ class LayerArtist(_LayerArtist):
     def update(self, view=None):
         pass
 
+    def clear(self):
+        pass
+
+    def redraw(self):
+        pass
+
 
 def setup_model(num):
-    ax = MagicMock()
-    mgrs = [LayerArtist(Data(label=str(i)), ax) for i in range(num)]
+    mgrs = [LayerArtist(Data(label=str(i))) for i in range(num)]
     model = LayerArtistModel(mgrs)
     return model, mgrs
 
@@ -35,16 +40,16 @@ def test_row_label():
 
 
 def test_add_artist_updates_row_count():
-    mgrs = [LayerArtist(Data(label='A'), None)]
+    mgrs = [LayerArtist(Data(label='A'))]
     model = LayerArtistModel(mgrs)
-    model.add_artist(0, LayerArtist(Data(label='B'), None))
+    model.add_artist(0, LayerArtist(Data(label='B')))
     assert model.rowCount() == 2
 
 
 def test_add_artist_updates_artist_list():
-    mgrs = [LayerArtist(Data(label='A'), None)]
+    mgrs = [LayerArtist(Data(label='A'))]
     model = LayerArtistModel(mgrs)
-    model.add_artist(0, LayerArtist(Data(label='B'), None))
+    model.add_artist(0, LayerArtist(Data(label='B')))
     assert len(mgrs) == 2
 
 
@@ -92,7 +97,7 @@ def test_change_label_invalid_row():
 
 
 def test_flags():
-    model, _ = setup_model(1)
+    model, layer_artists = setup_model(1)
 
     expected = (Qt.ItemIsEditable |
                 Qt.ItemIsDragEnabled |
@@ -115,8 +120,7 @@ def test_move_artist_empty():
 
 
 def test_move_artist_single():
-    ax = MagicMock()
-    m0 = LayerArtist(Data(label="test 0"), ax)
+    m0 = LayerArtist(Data(label="test 0"))
     mgrs = [m0]
 
     model = LayerArtistModel(mgrs)
@@ -170,9 +174,9 @@ def test_move_artist_three():
 
 
 def test_move_updates_zorder():
-    m0 = LayerArtist(Data(label='test 0'), MagicMock())
-    m1 = LayerArtist(Data(label='test 1'), MagicMock())
-    m2 = LayerArtist(Data(label='test 2'), MagicMock())
+    m0 = LayerArtist(Data(label='test 0'))
+    m1 = LayerArtist(Data(label='test 1'))
+    m2 = LayerArtist(Data(label='test 2'))
     m0.zorder = 10
     m1.zorder = 20
     m2.zorder = 30
@@ -187,7 +191,7 @@ def test_move_updates_zorder():
 
 
 def test_check_syncs_to_visible():
-    m0 = LayerArtist(Data(label='test 0'), MagicMock())
+    m0 = LayerArtist(Data(label='test 0'))
     m0.artists = [MagicMock()]
     mgrs = [m0]
     model = LayerArtistModel(mgrs)

glue/dialogs/data_wizard/qt/data_wizard_dialog.py

@@ -120,7 +120,8 @@ def load_data(self):
                 self._curfile = path
                 d = load_data(path, factory=fac.function)
                 if not isinstance(d, list):
-                    d.label = data_label(path)
+                    if not d.label:
+                        d.label = data_label(path)
                     d = [d]
                 result.extend(d)
 

glue/icons/qt/helpers.py

@@ -2,8 +2,10 @@
 
 from qtpy.QtCore import Qt
 from qtpy import QtWidgets, QtGui
-from glue.utils.qt import mpl_to_qt4_color, tint_pixmap
 
+from matplotlib.colors import Colormap
+
+from glue.utils.qt import mpl_to_qt4_color, tint_pixmap, cmap2pixmap
 from glue.icons import icon_path
 
 __all__ = ['symbol_icon', 'layer_icon', 'layer_artist_icon', 'get_icon', 'POINT_ICONS']
@@ -47,16 +49,19 @@ def layer_artist_icon(artist):
 
     from glue.viewers.scatter.layer_artist import ScatterLayerArtist
 
-    if not artist.enabled:
-        bm = QtGui.QBitmap(icon_path('glue_delete'))
-    elif isinstance(artist, ScatterLayerArtist):
-        bm = QtGui.QBitmap(icon_path(POINT_ICONS.get(artist.layer.style.marker,
-                                                     'glue_circle_point')))
+    color = artist.get_layer_color()
+
+    if isinstance(color, Colormap):
+        pm = cmap2pixmap(color)
     else:
-        bm = QtGui.QBitmap(icon_path('glue_box_point'))
-    color = mpl_to_qt4_color(artist.layer.style.color)
+        if isinstance(artist, ScatterLayerArtist):
+            bm = QtGui.QBitmap(icon_path(POINT_ICONS.get(artist.layer.style.marker,
+                                                         'glue_circle_point')))
+        else:
+            bm = QtGui.QBitmap(icon_path('glue_box_point'))
+        color = mpl_to_qt4_color(color)
+        pm = tint_pixmap(bm, color)
 
-    pm = tint_pixmap(bm, color)
     return QtGui.QIcon(pm)
 
 

glue/viewers/common/qt/mouse_mode.py

@@ -196,6 +196,8 @@ class RoiMode(RoiModeBase):
     mouse release
     """
 
+    status_tip = "CLICK and DRAG to define selection"
+
     def __init__(self, viewer, **kwargs):
 
         super(RoiMode, self).__init__(viewer, **kwargs)

glue/viewers/histogram/state.py

@@ -122,15 +122,7 @@ def bins(self):
 
     @defer_draw
     def _layers_changed(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.x_att_helper.set_multiple_data(datasets)
+        self.x_att_helper.set_multiple_data(self.layers_data)
 
 
 class HistogramLayerState(MatplotlibLayerState):

glue/viewers/image/layer_artist.py

@@ -122,6 +122,12 @@ def __init__(self, axes, viewer_state, layer_state=None, layer=None):
                            shape=self.get_image_shape)
         self.composite_image = self.axes._composite_image
 
+    def get_layer_color(self):
+        if self._viewer_state.color_mode == 'One color per layer':
+            return self.state.color
+        else:
+            return self.state.cmap
+
     def enable(self):
         if hasattr(self, 'composite_image'):
             self.composite_image.invalidate_cache()