From 4a63171773e829c97570750fd4502045c81dc60b Mon Sep 17 00:00:00 2001 From: Thomas Robitaille Date: Mon, 9 Oct 2017 18:33:09 +0100 Subject: [PATCH 1/3] Don't show Subsets in data collection view in the top left, instead make it so subset groups can be dragged and dropped onto viewers --- glue/core/application_base.py | 10 +++--- glue/core/qt/data_collection_model.py | 49 +++------------------------ 2 files changed, 10 insertions(+), 49 deletions(-) diff --git a/glue/core/application_base.py b/glue/core/application_base.py index aaaba3a08..28eb76a0e 100644 --- a/glue/core/application_base.py +++ b/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 diff --git a/glue/core/qt/data_collection_model.py b/glue/core/qt/data_collection_model.py index b877fefbc..ef8cf0f9e 100644 --- a/glue/core/qt/data_collection_model.py +++ b/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,12 +223,12 @@ 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: From c8f8f30627342966091d8a21337209e1fba937c2 Mon Sep 17 00:00:00 2001 From: Thomas Robitaille Date: Mon, 9 Oct 2017 18:33:44 +0100 Subject: [PATCH 2/3] Make it so that GroupedSubsets don't have their own style but instead defer to their parent SubsetGroup, which avoids synchronization issues --- glue/core/subset.py | 12 +++----- glue/core/subset_group.py | 65 +++++++++++++-------------------------- 2 files changed, 26 insertions(+), 51 deletions(-) diff --git a/glue/core/subset.py b/glue/core/subset.py index 213979fa3..cd97dfd0c 100644 --- a/glue/core/subset.py +++ b/glue/core/subset.py @@ -61,21 +61,17 @@ 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.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 diff --git a/glue/core/subset_group.py b/glue/core/subset_group.py index d7bf81858..709e912ab 100644 --- a/glue/core/subset_group.py +++ b/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,19 +157,6 @@ 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 @@ -175,19 +164,9 @@ def style(self): @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) From 4bfb8244a2cd0c09ec875a0ca928efee6aae0634 Mon Sep 17 00:00:00 2001 From: Thomas Robitaille Date: Mon, 9 Oct 2017 23:05:32 +0100 Subject: [PATCH 3/3] Remove option to have decoupled subset colors and fix tests --- glue/app/qt/preferences.py | 3 -- glue/app/qt/preferences.ui | 7 ----- glue/config.py | 1 - glue/core/data.py | 1 + glue/core/qt/data_collection_model.py | 10 ++++-- .../qt/tests/test_data_collection_model.py | 15 ++++----- glue/core/subset.py | 7 +++-- glue/core/tests/test_subset_group.py | 31 ------------------- glue/viewers/scatter/qt/layer_style_editor.ui | 3 ++ 9 files changed, 22 insertions(+), 56 deletions(-) diff --git a/glue/app/qt/preferences.py b/glue/app/qt/preferences.py index 857669e51..cc7ff8997 100644 --- a/glue/app/qt/preferences.py +++ b/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() diff --git a/glue/app/qt/preferences.ui b/glue/app/qt/preferences.ui index 7dd0881fc..4e419f74e 100644 --- a/glue/app/qt/preferences.ui +++ b/glue/app/qt/preferences.ui @@ -186,13 +186,6 @@ - - - - - - - diff --git a/glue/config.py b/glue/config.py index 018cead2a..2d4c20066 100644 --- a/glue/config.py +++ b/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) diff --git a/glue/core/data.py b/glue/core/data.py index f4c66dd59..0f15fea21 100644 --- a/glue/core/data.py +++ b/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): diff --git a/glue/core/qt/data_collection_model.py b/glue/core/qt/data_collection_model.py index ef8cf0f9e..0f472fd47 100644 --- a/glue/core/qt/data_collection_model.py +++ b/glue/core/qt/data_collection_model.py @@ -230,9 +230,12 @@ def index(self, row, column, parent=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() @@ -346,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: diff --git a/glue/core/qt/tests/test_data_collection_model.py b/glue/core/qt/tests/test_data_collection_model.py index 8ddad7e43..a4389d867 100644 --- a/glue/core/qt/tests/test_data_collection_model.py +++ b/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 diff --git a/glue/core/subset.py b/glue/core/subset.py index cd97dfd0c..17275ef14 100644 --- a/glue/core/subset.py +++ b/glue/core/subset.py @@ -67,13 +67,13 @@ def __init__(self, data, color=settings.SUBSET_COLORS[0], alpha=0.5, label=None) self.data = data 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): return self._subset_state @@ -285,6 +285,7 @@ def broadcast(self, attribute): :type attribute: ``str`` """ + if not hasattr(self, 'data') or not hasattr(self.data, 'hub'): return @@ -398,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) diff --git a/glue/core/tests/test_subset_group.py b/glue/core/tests/test_subset_group.py index 28b87ae06..58ef564e3 100644 --- a/glue/core/tests/test_subset_group.py +++ b/glue/core/tests/test_subset_group.py @@ -50,47 +50,16 @@ def test_attributes_synced_to_group(self): assert sub.subset_state is sg.subset_state assert sub.label is sg.label - @restore_settings - def test_set_style_overrides(self): - - # Test to make sure that if the user has selected to allow individual - # subset colors, the subset color can become out of sync with the - # group color. - - settings.INDIVIDUAL_SUBSET_COLOR = True - - self.sg.register(self.dc) - sg = self.sg - sg.subsets[0].style.color = 'blue' - for sub in sg.subsets[1:]: - assert sub.style.color != 'blue' - - assert sg.subsets[0].style.color == 'blue' - def test_new_subset_group_syncs_style(self): sg = self.dc.new_subset_group() for sub in sg.subsets: assert sub.style == sg.style - @restore_settings - def test_set_group_style_clears_override(self): - settings.INDIVIDUAL_SUBSET_COLOR = True - sg = self.dc.new_subset_group() - style = sg.style.copy() - style.parent = sg.subsets[0] - sg.subsets[0].style = style - style.color = 'blue' - sg.style.color = 'red' - assert sg.subsets[0].style.color == 'red' - def test_changing_subset_style_changes_group(self): # Test to make sure that if a subset's visual properties are changed, # the visual properties of all subsets in the same subset group are changed - # This is just to make sure the default setting is still False - assert not settings.INDIVIDUAL_SUBSET_COLOR - d1 = Data(x=[1, 2, 3], label='d1') d2 = Data(y=[2, 3, 4], label='d2') d3 = Data(y=[2, 3, 4], label='d3') diff --git a/glue/viewers/scatter/qt/layer_style_editor.ui b/glue/viewers/scatter/qt/layer_style_editor.ui index 9077a1041..3f4984219 100644 --- a/glue/viewers/scatter/qt/layer_style_editor.ui +++ b/glue/viewers/scatter/qt/layer_style_editor.ui @@ -22,6 +22,9 @@ Qt::Horizontal + + 100 +