RE #27189 Refactor calibration info into separate class

scripts/Engineering/gui/engineering_diffraction/tabs/calibration/presenter.py

@@ -7,9 +7,11 @@
 # pylint: disable=invalid-name
 from __future__ import (absolute_import, division, print_function)
 
+from copy import deepcopy
 from qtpy.QtWidgets import QMessageBox
 
 from Engineering.gui.engineering_diffraction.tabs.common import INSTRUMENT_DICT
+from Engineering.gui.engineering_diffraction.tabs.common.calibration_info import CalibrationInfo
 from mantidqt.utils.asynchronous import AsyncTask
 from mantid.simpleapi import logger
 from mantidqt.utils.observer_pattern import Observable
@@ -22,12 +24,13 @@ class CalibrationPresenter(object):
         self.worker = None
         self.calibration_notifier = self.CalibrationNotifier(self)
 
-        self.current_calibration = {"vanadium_path": None, "ceria_path": None, "instrument": None}
-        self.pending_calibration = {"vanadium_path": None, "ceria_path": None, "instrument": None}
+        self.current_calibration = CalibrationInfo()
+        self.pending_calibration = CalibrationInfo()
 
         # Connect view signals to local functions.
         self.view.set_on_calibrate_clicked(self.on_calibrate_clicked)
         self.view.set_enable_controls_connection(self.set_calibrate_controls_enabled)
+        self.view.set_update_fields_connection(self.set_field_values)
         self.view.set_on_radio_new_toggled(self.set_create_new_enabled)
         self.view.set_on_radio_existing_toggled(self.set_load_existing_enabled)
 
@@ -53,9 +56,7 @@ class CalibrationPresenter(object):
                 return
             filename = self.view.get_path_filename()
             instrument, vanadium_no, calib_no = self.model.load_existing_gsas_parameters(filename)
-            self.pending_calibration["vanadium_path"] = vanadium_no
-            self.pending_calibration["ceria_path"] = calib_no
-            self.pending_calibration["instrument"] = instrument
+            self.pending_calibration.set_calibration(vanadium_no, calib_no, instrument)
             self.set_current_calibration()
 
     def start_calibration_worker(self, vanadium_path, calib_path, plot_output, rb_num):
@@ -73,9 +74,7 @@ class CalibrationPresenter(object):
         },
                                 error_cb=self._on_error,
                                 success_cb=self._on_success)
-        self.pending_calibration["vanadium_path"] = vanadium_path
-        self.pending_calibration["ceria_path"] = calib_path
-        self.pending_calibration["instrument"] = self.instrument
+        self.pending_calibration.set_calibration(vanadium_path, calib_path, self.instrument)
         self.set_calibrate_controls_enabled(False)
         self.worker.start()
 
@@ -85,14 +84,14 @@ class CalibrationPresenter(object):
     def set_current_calibration(self, success_info=None):
         if success_info:
             logger.information("Thread executed in " + str(success_info.elapsed_time) + " seconds.")
-        self.current_calibration = self.pending_calibration
+        self.current_calibration = deepcopy(self.pending_calibration)
         self.calibration_notifier.notify_subscribers(self.current_calibration)
-        self.set_field_values()
-        self.pending_calibration = {"vanadium_path": None, "ceria_path": None, "instrument": None}
+        self.emit_update_fields_signal()
+        self.pending_calibration.clear()
 
     def set_field_values(self):
-        self.view.set_calib_text(self.current_calibration["ceria_path"])
-        self.view.set_vanadium_text(self.current_calibration["vanadium_path"])
+        self.view.set_calib_text(self.current_calibration.get_ceria())
+        self.view.set_vanadium_text(self.current_calibration.get_vanadium())
 
     def set_instrument_override(self, instrument):
         instrument = INSTRUMENT_DICT[instrument]
@@ -114,6 +113,9 @@ class CalibrationPresenter(object):
     def emit_enable_button_signal(self):
         self.view.sig_enable_controls.emit(True)
 
+    def emit_update_fields_signal(self):
+        self.view.sig_update_fields.emit()
+
     def set_calibrate_controls_enabled(self, enabled):
         self.view.set_calibrate_button_enabled(enabled)
         self.view.set_check_plot_output_enabled(enabled)

scripts/Engineering/gui/engineering_diffraction/tabs/calibration/test/test_calib_presenter.py

@@ -12,6 +12,7 @@ import unittest
 from mantid.py3compat.mock import patch, MagicMock
 from mantid.py3compat import mock
 from Engineering.gui.engineering_diffraction.tabs.calibration import model, view, presenter
+from Engineering.gui.engineering_diffraction.tabs.common.calibration_info import CalibrationInfo
 
 tab_path = 'Engineering.gui.engineering_diffraction.tabs.calibration'
 
@@ -115,58 +116,44 @@ class CalibrationPresenterTest(unittest.TestCase):
         self.view.set_instrument_override.assert_called_with("IMAT")
         self.assertEqual(self.presenter.instrument, "IMAT")
 
-    def test_set_current_calibration(self):
+    @patch(tab_path + ".presenter.CalibrationPresenter.emit_update_fields_signal")
+    def test_set_current_calibration(self, update_sig):
         self.presenter.calibration_notifier = MagicMock()
-        pending = {
-            "vanadium_path": "/test/set/path",
-            "ceria_path": "test/set/path/2",
-            "instrument": "TEST_INS"
-        }
+        pending = CalibrationInfo(vanadium_path="/test/set/path", ceria_path="test/set/path/2", instrument="TEST_INS")
+        pendcpy = CalibrationInfo(vanadium_path="/test/set/path", ceria_path="test/set/path/2", instrument="TEST_INS")
         self.presenter.pending_calibration = pending
-        current = {
-            "vanadium_path": "old/value",
-            "ceria_path": "old/cera",
-            "instrument": "ENGINX"
-        }
-        blank = {
-            "vanadium_path": None,
-            "ceria_path": None,
-            "instrument": None
-        }
+        current = CalibrationInfo(vanadium_path="old/value", ceria_path="old/cera", instrument="ENGINX")
+        blank = CalibrationInfo()
         self.presenter.current_calibration = current
 
         self.assertEqual(self.presenter.current_calibration, current)
         self.presenter.set_current_calibration()
 
-        self.assertEqual(self.presenter.current_calibration, pending)
-        self.assertEqual(self.presenter.pending_calibration, blank)
+        self.check_calibration_equal(self.presenter.current_calibration, pendcpy)
+        self.check_calibration_equal(self.presenter.pending_calibration, blank)
         self.assertEqual(self.presenter.calibration_notifier.notify_subscribers.call_count, 1)
-        self.assertEqual(self.view.set_vanadium_text.call_count, 1)
-        self.assertEqual(self.view.set_calib_text.call_count, 1)
+        self.assertEqual(update_sig.call_count, 1)
 
+    @patch(tab_path + ".presenter.CalibrationPresenter.emit_update_fields_signal")
     @patch(tab_path + ".presenter.CalibrationPresenter.validate_path")
-    def test_calibrate_clicked_load_valid_path(self, path):
+    def test_calibrate_clicked_load_valid_path(self, path, update):
         self.presenter.calibration_notifier = MagicMock()
         self.view.get_new_checked.return_value = False
         self.view.get_load_checked.return_value = True
+
         path.return_value = True
         instrument, van, cer = ("test_ins", "test_van", "test_cer")
         self.model.load_existing_gsas_parameters.return_value = instrument, van, cer
-        current = {
-            "vanadium_path": "old/value",
-            "ceria_path": "old/cera",
-            "instrument": "ENGINX"
-        }
-        new = {
-            "vanadium_path": van,
-            "ceria_path": cer,
-            "instrument": instrument
-        }
+        current = CalibrationInfo(vanadium_path="old/value", ceria_path="old/cera", instrument="ENGINX")
+        new = CalibrationInfo(vanadium_path=van, ceria_path=cer, instrument=instrument)
         self.presenter.current_calibration = current
 
         self.presenter.on_calibrate_clicked()
 
-        self.assertEqual(self.presenter.current_calibration, new)
+        self.assertEqual(update.call_count, 1)
+        self.assertEqual(self.presenter.current_calibration.get_vanadium(), new.get_vanadium())
+        self.assertEqual(self.presenter.current_calibration.get_ceria(), new.get_ceria())
+        self.assertEqual(self.presenter.current_calibration.get_instrument(), new.get_instrument())
         self.assertEqual(self.presenter.calibration_notifier.notify_subscribers.call_count, 1)
 
     @patch(tab_path + ".presenter.CalibrationPresenter.validate_path")
@@ -175,11 +162,7 @@ class CalibrationPresenterTest(unittest.TestCase):
         self.view.get_new_checked.return_value = False
         self.view.get_load_checked.return_value = True
         path.return_value = False
-        current = {
-            "vanadium_path": "old/value",
-            "ceria_path": "old/cera",
-            "instrument": "ENGINX"
-        }
+        current = CalibrationInfo(vanadium_path="old/value", ceria_path="old/cera", instrument="ENGINX")
         self.presenter.current_calibration = current
 
         self.presenter.on_calibrate_clicked()
@@ -228,22 +211,19 @@ class CalibrationPresenterTest(unittest.TestCase):
     @patch(tab_path + ".presenter.AsyncTask")
     def test_start_calibration_worker(self, task):
         instrument, van, cer = ("test_ins", "test_van", "test_cer")
-        old_pending = {
-            "vanadium_path": None,
-            "ceria_path": None,
-            "instrument": None
-        }
+        old_pending = CalibrationInfo(vanadium_path=None, ceria_path=None, instrument=None)
         self.presenter.pending_calibration = old_pending
-        expected_pending = {
-            "vanadium_path": van,
-            "ceria_path": cer,
-            "instrument": instrument
-        }
+        expected_pending = CalibrationInfo(vanadium_path=van, ceria_path=cer, instrument=instrument)
         self.presenter.instrument = instrument
 
         self.presenter.start_calibration_worker(van, cer, False, None)
 
-        self.assertEqual(self.presenter.pending_calibration, expected_pending)
+        self.check_calibration_equal(self.presenter.pending_calibration, expected_pending)
+
+    def check_calibration_equal(self, a, b):
+        self.assertEqual(a.get_vanadium(), b.get_vanadium())
+        self.assertEqual(a.get_ceria(), b.get_ceria())
+        self.assertEqual(a.get_instrument(), b.get_instrument())
 
 
 if __name__ == '__main__':

scripts/Engineering/gui/engineering_diffraction/tabs/calibration/view.py

@@ -15,6 +15,7 @@ Ui_calib, _ = load_ui(__file__, "calibration_tab.ui")
 
 class CalibrationView(QtWidgets.QWidget, Ui_calib):
     sig_enable_controls = QtCore.Signal(bool)
+    sig_update_fields = QtCore.Signal()
 
     def __init__(self, parent=None, instrument="ENGINX"):
         super(CalibrationView, self).__init__(parent)
@@ -54,6 +55,9 @@ class CalibrationView(QtWidgets.QWidget, Ui_calib):
     def set_enable_controls_connection(self, slot):
         self.sig_enable_controls.connect(slot)
 
+    def set_update_fields_connection(self, slot):
+        self.sig_update_fields.connect(slot)
+
     # =================
     # Component Setters
     # =================

scripts/Engineering/gui/engineering_diffraction/tabs/common/calibration_info.py

+# Mantid Repository : https://github.com/mantidproject/mantid
+#
+# Copyright © 2019 ISIS Rutherford Appleton Laboratory UKRI,
+#     NScD Oak Ridge National Laboratory, European Spallation Source
+#     & Institut Laue - Langevin
+# SPDX - License - Identifier: GPL - 3.0 +
+
+from __future__ import (absolute_import, division, print_function)
+
+
+class CalibrationInfo(object):
+    """
+    Keeps track of the parameters that went into a calibration created by the engineering diffraction GUI.
+    """
+    def __init__(self, vanadium_path=None, ceria_path=None, instrument=None):
+        self.vanadium_path = vanadium_path
+        self.ceria_path = ceria_path
+        self.instrument = instrument
+
+    def set_calibration(self, vanadium_path, ceria_path, instrument):
+        """
+        Set the values of the calibration. requires a complete set of calibration info to be supplied.
+        :param vanadium_path: Path to the vanadium data file used in the calibration.
+        :param ceria_path: Path to the ceria data file used.
+        :param instrument: String defining the instrument the data came from.
+        """
+        self.vanadium_path = vanadium_path
+        self.ceria_path = ceria_path
+        self.instrument = instrument
+
+    def get_vanadium(self):
+        return self.vanadium_path
+
+    def get_ceria(self):
+        return self.ceria_path
+
+    def get_instrument(self):
+        return self.instrument
+
+    def clear(self):
+        self.vanadium_path = None
+        self.ceria_path = None
+        self.instrument = None
+
+    def is_valid(self):
+        return True if self.vanadium_path and self.ceria_path and self.instrument else False
+

scripts/Engineering/gui/engineering_diffraction/tabs/focus/presenter.py

@@ -11,6 +11,7 @@ from qtpy.QtWidgets import QMessageBox
 
 from Engineering.gui.engineering_diffraction.tabs.common import INSTRUMENT_DICT
 from Engineering.gui.engineering_diffraction.tabs.common.vanadium_corrections import check_workspaces_exist
+from Engineering.gui.engineering_diffraction.tabs.common.calibration_info import CalibrationInfo
 from mantidqt.utils.asynchronous import AsyncTask
 from mantidqt.utils.observer_pattern import Observer
 from mantid.simpleapi import logger
@@ -28,7 +29,7 @@ class FocusPresenter(object):
         self.view.set_enable_controls_connection(self.set_focus_controls_enabled)
 
         # Variables from other GUI tabs.
-        self.current_calibration = {"vanadium_path": None, "ceria_path": None}
+        self.current_calibration = CalibrationInfo()
         self.instrument = "ENGINX"
         self.rb_num = None
 
@@ -70,10 +71,15 @@ class FocusPresenter(object):
         """
         if not self.view.get_focus_valid():
             return False
-        if self.current_calibration["vanadium_path"] is None or not check_workspaces_exist():
+        if not self.current_calibration.is_valid():
             self._create_error_message(
-                "Load a calibration from the Calibration tab before focusing.")
+                "Create or Load a calibration via the Calibration tab before focusing.")
             return False
+        if self.current_calibration.get_instrument() != self.instrument:
+            self._create_error_message(
+                "Please make sure the selected instrument matches instrument for the current calibration.\n"
+                "The instrument for the calibration is: " +
+                self.current_calibration.get_instrument())
         if self.view.is_searching():
             return False
         if len(banks) == 0:

scripts/Engineering/gui/engineering_diffraction/tabs/focus/test/test_focus_presenter.py

@@ -12,6 +12,7 @@ import unittest
 from mantid.py3compat import mock
 from mantid.py3compat.mock import patch
 from Engineering.gui.engineering_diffraction.tabs.focus import model, view, presenter
+from Engineering.gui.engineering_diffraction.tabs.common.calibration_info import CalibrationInfo
 
 tab_path = "Engineering.gui.engineering_diffraction.tabs.focus"
 
@@ -25,10 +26,9 @@ class FocusPresenterTest(unittest.TestCase):
     @patch(tab_path + ".presenter.check_workspaces_exist")
     @patch(tab_path + ".presenter.FocusPresenter.start_focus_worker")
     def test_worker_started_with_correct_params(self, worker, wsp_exists):
-        self.presenter.current_calibration = {
-            "vanadium_path": "Fake/Path",
-            "ceria_path": "Fake/Path"
-        }
+        self.presenter.current_calibration = CalibrationInfo(vanadium_path="Fake/Path",
+                                                             ceria_path="Fake/Path",
+                                                             instrument="ENGINX")
         self.view.get_focus_filename.return_value = "305738"
         self.view.get_north_bank.return_value = False
         self.view.get_south_bank.return_value = True
@@ -101,20 +101,21 @@ class FocusPresenterTest(unittest.TestCase):
 
     @patch(tab_path + ".presenter.FocusPresenter._create_error_message")
     def test_validate_with_invalid_calibration(self, create_error):
-        self.presenter.current_calibration = {"vanadium_path": None, "ceria_path": None}
+        self.presenter.current_calibration = CalibrationInfo(vanadium_path=None,
+                                                             ceria_path=None,
+                                                             instrument=None)
         banks = ["North", "South"]
 
         self.presenter._validate(banks)
         create_error.assert_called_with(
-            "Load a calibration from the Calibration tab before focusing.")
+            "Create or Load a calibration via the Calibration tab before focusing.")
 
     @patch(tab_path + ".presenter.check_workspaces_exist")
     @patch(tab_path + ".presenter.FocusPresenter._create_error_message")
     def test_validate_while_searching(self, create_error, wsp_check):
-        self.presenter.current_calibration = {
-            "vanadium_path": "Fake/File/Path",
-            "ceria_path": "Fake/Path"
-        }
+        self.presenter.current_calibration = CalibrationInfo(vanadium_path="Fake/File/Path",
+                                                             ceria_path="Fake/Path",
+                                                             instrument="ENGINX")
         self.view.is_searching.return_value = True
         wsp_check.return_value = True
         banks = ["North", "South"]
@@ -125,10 +126,9 @@ class FocusPresenterTest(unittest.TestCase):
     @patch(tab_path + ".presenter.check_workspaces_exist")
     @patch(tab_path + ".presenter.FocusPresenter._create_error_message")
     def test_validate_with_no_banks_selected(self, create_error, wsp_check):
-        self.presenter.current_calibration = {
-            "vanadium_path": "Fake/Path",
-            "ceria_path": "Fake/Path"
-        }
+        self.presenter.current_calibration = CalibrationInfo(vanadium_path="Fake/Path",
+                                                             ceria_path="Fake/Path",
+                                                             instrument="ENGINX")
         self.view.is_searching.return_value = False
         banks = []
         wsp_check.return_value = True