diff --git a/scripts/Muon/GUI/Common/grouping_tab_widget/grouping_tab_widget_presenter.py b/scripts/Muon/GUI/Common/grouping_tab_widget/grouping_tab_widget_presenter.py
index 50ac4a6f7a562553af0ef076acd28d4fb81a5f6e..58f3fb458c7fe7560d8ffe55eae0fcdab2573f1a 100644
--- a/scripts/Muon/GUI/Common/grouping_tab_widget/grouping_tab_widget_presenter.py
+++ b/scripts/Muon/GUI/Common/grouping_tab_widget/grouping_tab_widget_presenter.py
@@ -110,7 +110,7 @@ class GroupingTabPresenter(object):
for pair in pairs:
self._model.add_pair(pair)
except ValueError as error:
- self._view.display_warning_box(error)
+ self._view.display_warning_box(str(error))
self.grouping_table_widget.update_view_from_model()
self.pairing_table_widget.update_view_from_model()
diff --git a/scripts/Muon/GUI/Common/grouping_table_widget/grouping_table_widget_presenter.py b/scripts/Muon/GUI/Common/grouping_table_widget/grouping_table_widget_presenter.py
index f3212e7b4d1a5af19595fc68b7d7def09760ea26..bd10dde2469b1dab70460113b7123df6a9ed8af4 100644
--- a/scripts/Muon/GUI/Common/grouping_table_widget/grouping_table_widget_presenter.py
+++ b/scripts/Muon/GUI/Common/grouping_table_widget/grouping_table_widget_presenter.py
@@ -50,7 +50,6 @@ class GroupingTablePresenter(object):
return True
def validate_detector_ids(self, text):
- # detector_list = run_utils.run_string_to_list(text)
if re.match(run_utils.run_string_regex, text) and max(run_utils.run_string_to_list(text)) <= self._model._data.num_detectors:
return True
self._view.warning_popup("Invalid detector list.")
@@ -120,7 +119,6 @@ class GroupingTablePresenter(object):
try:
self.update_model_from_view()
except ValueError as error:
- self._model.remove_pairs_with_removed_name(self._view.get_table_item_text(row, 0))
self._view.warning_popup(error)
self.update_view_from_model()
diff --git a/scripts/Muon/GUI/Common/muon_data_context.py b/scripts/Muon/GUI/Common/muon_data_context.py
index c8f79d7e2abb755e842f1f8c4e4d0728873972a4..aa8baa7316afec818162c7db3954937b4d63c0f9 100644
--- a/scripts/Muon/GUI/Common/muon_data_context.py
+++ b/scripts/Muon/GUI/Common/muon_data_context.py
@@ -157,7 +157,7 @@ class MuonDataContext(object):
if self.check_group_contains_valid_detectors(group):
self._groups[group.name] = group
else:
- raise ValueError('Detectors in group {} not in instrument'.format(group.name))
+ raise ValueError('Invalid detectors in group {}'.format(group.name))
def add_pair(self, pair):
assert isinstance(pair, MuonPair)
@@ -303,7 +303,7 @@ class MuonDataContext(object):
self.add_pair(pair)
def check_group_contains_valid_detectors(self, group):
- if max(group.detectors) > self.num_detectors:
+ if max(group.detectors) > self.num_detectors or min(group.detectors) < 1:
return False
else:
return True
diff --git a/scripts/test/Muon/grouping_tab/grouping_tab_presenter_test.py b/scripts/test/Muon/grouping_tab/grouping_tab_presenter_test.py
index e0ee8a70876847539681427665784becbc7d9cc3..29dfd74fd746f3b8da998ed2832eea723af3e1ba 100644
--- a/scripts/test/Muon/grouping_tab/grouping_tab_presenter_test.py
+++ b/scripts/test/Muon/grouping_tab/grouping_tab_presenter_test.py
@@ -45,6 +45,8 @@ class GroupingTabPresenterTest(unittest.TestCase):
self.grouping_table_widget,
self.pairing_table_widget)
+ self.view.display_warning_box = mock.MagicMock()
+
def add_three_groups(self):
testgroup1 = MuonGroup(group_name="fwd", detector_ids=[1, 2, 3, 4, 5])
testgroup2 = MuonGroup(group_name="bwd", detector_ids=[6, 7, 8, 9, 10])
@@ -122,6 +124,23 @@ class GroupingTabPresenterTest(unittest.TestCase):
self.assertEqual(self.pairing_table_view.pairing_table.cellWidget(0, 1).currentText(), "grp1")
self.assertEqual(self.pairing_table_view.pairing_table.cellWidget(0, 2).currentText(), "grp2")
+ def test_loading_does_not_insert_invalid_groups(self):
+ self.view.show_file_browser_and_return_selection = mock.Mock(return_value="grouping.xml")
+ groups = [MuonGroup(group_name="grp1", detector_ids=[1, 2, 3, 4, 5]),
+ MuonGroup(group_name="grp2", detector_ids=[6, 7, 8, 9, 1000])]
+ pairs = [MuonPair(pair_name="pair1", forward_group_name="grp1", backward_group_name="grp2")]
+ with mock.patch(
+ "Muon.GUI.Common.grouping_tab_widget.grouping_tab_widget_presenter.xml_utils.load_grouping_from_XML") as mock_load:
+ # mock the loading to return set groups/pairs
+ mock_load.return_value = (groups, pairs, 'description')
+ self.view.load_grouping_button.clicked.emit(True)
+
+ self.view.display_warning_box.assert_called_once_with('Invalid detectors in group grp2')
+ six.assertCountEqual(self, self.model.group_names, ["grp1"])
+ six.assertCountEqual(self, self.model.pair_names, [])
+ self.assertEqual(self.grouping_table_view.num_rows(), 1)
+ self.assertEqual(self.pairing_table_view.num_rows(), 0)
+
def test_that_save_grouping_triggers_the_correct_function(self):
# Save functionality is tested elsewhere
self.view.show_file_save_browser_and_return_selection = mock.Mock(return_value="grouping.xml")
diff --git a/scripts/test/Muon/grouping_tab/grouping_table_presenter_test.py b/scripts/test/Muon/grouping_tab/grouping_table_presenter_test.py
index e999fb2fa2de06fa0305b8f7cb7ce497da360739..6a359d82cd6d80032d4b5e311c6a34411f2f3900 100644
--- a/scripts/test/Muon/grouping_tab/grouping_table_presenter_test.py
+++ b/scripts/test/Muon/grouping_tab/grouping_table_presenter_test.py
@@ -295,6 +295,17 @@ class GroupingTablePresenterTest(unittest.TestCase):
self.assertEqual(self.view.get_table_item_text(0, 2), "1")
+ def test_modifying_detector_ids_to_non_existent_detector_fails(self):
+ self.presenter.handle_add_group_button_clicked()
+ self.view.grouping_table.item(0, 1).setText("1000")
+
+ self.view.warning_popup.assert_called_once_with('Invalid detector list.')
+
+ def test_modifying_detector_ids_to_negative_detectors_fails(self):
+ self.presenter.handle_add_group_button_clicked()
+ self.view.grouping_table.item(0, 1).setText("-10-10")
+
+ self.view.warning_popup.assert_called_once_with('Invalid detector list.')
if __name__ == '__main__':
unittest.main(buffer=False, verbosity=2)