diff --git a/glue/core/data_collection.py b/glue/core/data_collection.py index 863a7baa8..95f21906f 100644 --- a/glue/core/data_collection.py +++ b/glue/core/data_collection.py @@ -269,9 +269,12 @@ def new_subset_group(self, label=None, subset_state=None, **kwargs): self._sg_count += 1 label = label or 'Subset %i' % self._sg_count - result = SubsetGroup(label=label, subset_state=subset_state, **kwargs) - self._subset_groups.append(result) - result.register(self) + # We delay callbacks so that SubsetCreateMessages are only emitted once + # the subset exists in all datasets. + with self.hub.delay_callbacks(): + result = SubsetGroup(label=label, subset_state=subset_state, **kwargs) + self._subset_groups.append(result) + result.register(self) return result def remove_subset_group(self, subset_grp): @@ -281,12 +284,13 @@ def remove_subset_group(self, subset_grp): if subset_grp not in self._subset_groups: return - # remove from list first, so that group appears deleted - # by the time the first SubsetDelete message is broadcast - self._subset_groups.remove(subset_grp) - for s in subset_grp.subsets: - s.delete() - subset_grp.unregister(self.hub) + # We delay callbacks so that SubsetDelteMessages are only emitted once + # the subset exists in all datasets. + with self.hub.delay_callbacks(): + self._subset_groups.remove(subset_grp) + for s in subset_grp.subsets: + s.delete() + subset_grp.unregister(self.hub) def suggest_merge_label(self, *data): """ diff --git a/glue/core/edit_subset_mode.py b/glue/core/edit_subset_mode.py index 9eb83ff83..3784f8cf0 100644 --- a/glue/core/edit_subset_mode.py +++ b/glue/core/edit_subset_mode.py @@ -71,7 +71,11 @@ def _combine_data(self, new_state, override_mode=None): if self.data_collection is None: raise RuntimeError("Must set data_collection before " "calling update") - self.edit_subset = [self.data_collection.new_subset_group()] + # We set the subset state directly here to avoid double messages + # (create then update). As the subset is new at this point, + # it doesn't make sense to apply a mode in any case. + self.edit_subset = [self.data_collection.new_subset_group(subset_state=new_state.copy())] + return subs = self._edit_subset for s in as_list(subs): mode(s, new_state) diff --git a/glue/core/tests/test_edit_subset_mode.py b/glue/core/tests/test_edit_subset_mode.py index 487ae395e..3875ecb47 100644 --- a/glue/core/tests/test_edit_subset_mode.py +++ b/glue/core/tests/test_edit_subset_mode.py @@ -1,12 +1,19 @@ # pylint: disable=I0011,W0613,W0201,W0212,E1101,E1103 +import operator +from unittest.mock import MagicMock + import numpy as np +from ..application_base import Application +from ..hub import HubListener +from ..message import SubsetCreateMessage, SubsetUpdateMessage, SubsetDeleteMessage +from ..command import ApplySubsetState from ..data import Component, Data from ..data_collection import DataCollection from ..edit_subset_mode import (EditSubsetMode, ReplaceMode, OrMode, AndMode, XorMode, AndNotMode) -from ..subset import ElementSubsetState +from ..subset import ElementSubsetState, InequalitySubsetState class TestEditSubsetMode(object): @@ -87,3 +94,122 @@ def test_combines_make_copy(self): self.edit_mode.edit_subset = self.data.new_subset() self.edit_mode.update(self.data, self.state2) assert self.edit_mode.edit_subset.subset_state is not self.state2 + + +def test_no_double_messaging(): + + # Make sure that when we create a new subset via EditSubsetMode, we don't + # get two separate messages for creation and updating, but instead just a + # single create message with the right subset state. + + handler = MagicMock() + + app = Application() + + class Listener(HubListener): + pass + + listener = Listener() + + app.session.hub.subscribe(listener, SubsetCreateMessage, handler=handler) + app.session.hub.subscribe(listener, SubsetUpdateMessage, handler=handler) + + data = Data(x=[1, 2, 3, 4, 5]) + + app.data_collection.append(data) + + cmd = ApplySubsetState(data_collection=app.data_collection, + subset_state=data.id['x'] >= 3, + override_mode=ReplaceMode) + + app.session.command_stack.do(cmd) + + assert handler.call_count == 1 + message = handler.call_args[0][0] + assert isinstance(message, SubsetCreateMessage) + assert isinstance(message.subset.subset_state, InequalitySubsetState) + assert message.subset.subset_state.left is data.id['x'] + assert message.subset.subset_state.operator is operator.ge + assert message.subset.subset_state.right == 3 + + +def test_broadcast_to_collection(): + """A data collection input works on each data component""" + + handler = MagicMock() + + app = Application() + + class Listener(HubListener): + pass + + listener = Listener() + + app.session.hub.subscribe(listener, SubsetCreateMessage, handler=handler) + app.session.hub.subscribe(listener, SubsetUpdateMessage, handler=handler) + + data = Data(x=[1, 2, 3, 4, 5]) + data2 = Data(x=[2, 3, 4, 5, 6]) + + app.data_collection.append(data) + app.data_collection.append(data2) + + cmd = ApplySubsetState(data_collection=app.data_collection, + subset_state=data.id['x'] >= 3, + override_mode=ReplaceMode) + + app.session.command_stack.do(cmd) + + assert len(data2.subsets) == 1 + assert data2.subsets[0].label == 'Subset 1' + # fails with `IncompatibleAttribute` exception + # assert data2.get_subset_object(subset_id='Subset 1', cls=NDDataArray) + + assert handler.call_count == 2 + + assert data2.subsets[0].subset_state.left is data.id['x'] + assert data2.subsets[0].subset_state.operator is operator.ge + assert data2.subsets[0].subset_state.right == 3 + + +def test_message_once_all_subsets_exist(): + + # Make sure that when we create a new subset via EditSubsetMode, we don't + # get two separate messages for creation and updating, but instead just a + # single create message with the right subset state. + + app = Application() + + class Listener(HubListener): + pass + + listener = Listener() + + data1 = Data(x=[1, 2, 3, 4, 5]) + app.data_collection.append(data1) + + data2 = Data(x=[1, 2, 3, 4, 5]) + app.data_collection.append(data2) + + count = [0] + + def handler(msg): + assert len(data1.subsets) == len(data2.subsets) + for i in range(len(data1.subsets)): + assert data2.subsets[i].subset_state is data1.subsets[i].subset_state + count[0] += 1 + + app.session.hub.subscribe(listener, SubsetCreateMessage, handler=handler) + app.session.hub.subscribe(listener, SubsetDeleteMessage, handler=handler) + + cmd = ApplySubsetState(data_collection=app.data_collection, + subset_state=data1.id['x'] >= 3, + override_mode=ReplaceMode) + + app.session.command_stack.do(cmd) + + assert count[0] == 2 + + app.data_collection.remove_subset_group(app.data_collection.subset_groups[0]) + + assert count[0] == 4