From fdc97b20598a77bc6e28c89797c204204c046e6e Mon Sep 17 00:00:00 2001
From: Conor Finn <conor.finn@stfc.ac.uk>
Date: Tue, 22 Oct 2019 13:17:00 +0100
Subject: [PATCH] RE #27012 Add method documentation and make methods private
Documentation has been added for the less obvious public and private methods.
Some of the methods in the model should be shown as private, so have
had methods refactored.
---
.../tabs/calibration/model.py | 9 ++--
.../tabs/calibration/test/test_calib_model.py | 3 +-
.../tabs/focus/model.py | 32 ++++++++++---
.../tabs/focus/presenter.py | 48 +++++++++++++------
.../tabs/focus/test/test_focus_model.py | 16 +++----
.../tabs/focus/test/test_focus_presenter.py | 46 +++++++++++-------
6 files changed, 101 insertions(+), 53 deletions(-)
diff --git a/scripts/Engineering/gui/engineering_diffraction/tabs/calibration/model.py b/scripts/Engineering/gui/engineering_diffraction/tabs/calibration/model.py
index b323a977932..79044a438dd 100644
--- a/scripts/Engineering/gui/engineering_diffraction/tabs/calibration/model.py
+++ b/scripts/Engineering/gui/engineering_diffraction/tabs/calibration/model.py
@@ -18,7 +18,6 @@ from Engineering.EnggUtils import write_ENGINX_GSAS_iparam_file
from Engineering.gui.engineering_diffraction.tabs.common import vanadium_corrections
from Engineering.gui.engineering_diffraction.tabs.common import path_handling
-
VANADIUM_INPUT_WORKSPACE_NAME = "engggui_vanadium_ws"
CURVES_WORKSPACE_NAME = "engggui_vanadium_curves"
INTEGRATED_WORKSPACE_NAME = "engggui_vanadium_integration"
@@ -45,7 +44,8 @@ class CalibrationModel(object):
:param instrument: The instrument the data relates to.
:param rb_num: The RB number for file creation.
"""
- van_integration, van_curves = vanadium_corrections.fetch_correction_workspaces(vanadium_path, instrument)
+ van_integration, van_curves = vanadium_corrections.fetch_correction_workspaces(
+ vanadium_path, instrument)
ceria_workspace = self.load_ceria(ceria_path)
output = self.run_calibration(ceria_workspace, van_integration, van_curves)
if plot_output:
@@ -65,7 +65,8 @@ class CalibrationModel(object):
self.create_output_files(CALIBRATION_DIR, difc, tzero, ceria_path, vanadium_path,
instrument)
if rb_num:
- user_calib_dir = path.join(path_handling.OUT_FILES_ROOT_DIR, "User", rb_num, "Calibration", "")
+ user_calib_dir = path.join(path_handling.OUT_FILES_ROOT_DIR, "User", rb_num,
+ "Calibration", "")
self.create_output_files(user_calib_dir, difc, tzero, ceria_path, vanadium_path,
instrument)
@@ -109,7 +110,7 @@ class CalibrationModel(object):
@staticmethod
def _plot_difc_zero(difc, tzero):
for i in range(1, 3):
- bank_ws = Ads.retrieve(CalibrationModel._generate_table_workspace_name(i-1))
+ bank_ws = Ads.retrieve(CalibrationModel._generate_table_workspace_name(i - 1))
x_val = []
y_val = []
diff --git a/scripts/Engineering/gui/engineering_diffraction/tabs/calibration/test/test_calib_model.py b/scripts/Engineering/gui/engineering_diffraction/tabs/calibration/test/test_calib_model.py
index 19177bd46b2..f75ef6cecd1 100644
--- a/scripts/Engineering/gui/engineering_diffraction/tabs/calibration/test/test_calib_model.py
+++ b/scripts/Engineering/gui/engineering_diffraction/tabs/calibration/test/test_calib_model.py
@@ -47,7 +47,8 @@ class CalibrationModelTest(unittest.TestCase):
@patch(class_path + '.load_ceria')
@patch(class_path + '.run_calibration')
@patch(file_path + '.vanadium_corrections.fetch_correction_workspaces')
- def test_fetch_vanadium_is_called(self, van_corr, calibrate_alg, load_ceria, output_files, update_table):
+ def test_fetch_vanadium_is_called(self, van_corr, calibrate_alg, load_ceria, output_files,
+ update_table):
van_corr.return_value = ("mocked_integration", "mocked_curves")
self.model.create_new_calibration(VANADIUM_NUMBER, CERIUM_NUMBER, False, "ENGINX")
self.assertEqual(van_corr.call_count, 1)
diff --git a/scripts/Engineering/gui/engineering_diffraction/tabs/focus/model.py b/scripts/Engineering/gui/engineering_diffraction/tabs/focus/model.py
index 9004c06b981..878c79155bb 100644
--- a/scripts/Engineering/gui/engineering_diffraction/tabs/focus/model.py
+++ b/scripts/Engineering/gui/engineering_diffraction/tabs/focus/model.py
@@ -19,6 +19,15 @@ FOCUSED_OUTPUT_WORKSPACE_NAME = "engggui_focusing_output_ws_bank_"
class FocusModel(object):
def focus_run(self, sample_path, banks, plot_output, instrument, rb_number, current_calib):
+ """
+ Focus some data using the current calibration.
+ :param sample_path: The path to the data to be focused.
+ :param banks: The banks that should be focused.
+ :param plot_output: True if the output should be plotted.
+ :param instrument: The instrument that the data came from.
+ :param rb_number: The experiment number, used to create directories. Can be None
+ :param current_calib: The current calibration loaded in the calibration tab.
+ """
vanadium_path = current_calib["vanadium_path"]
if vanadium_path is None:
return
@@ -33,8 +42,8 @@ class FocusModel(object):
if plot_output:
self._plot_focused_workspace(output_workspace_name)
# Save the output to the file system.
- self.save_focused_output_files_as_nexus(instrument, sample_path, name,
- output_workspace_name, rb_number)
+ self._save_focused_output_files_as_nexus(instrument, sample_path, name,
+ output_workspace_name, rb_number)
@staticmethod
def _run_focus(input_workspace, output_workspace, vanadium_integration_ws, vanadium_curves_ws,
@@ -66,19 +75,28 @@ class FocusModel(object):
focused_wsp = Ads.retrieve(focused)
plot([focused_wsp], wksp_indices=[0])
- def save_focused_output_files_as_nexus(self, instrument, sample_path, bank, sample_workspace,
- rb_number):
+ def _save_focused_output_files_as_nexus(self, instrument, sample_path, bank, sample_workspace,
+ rb_number):
+ """
+ Save a focused workspace to the file system. Saves a separate copy to a User directory if an rb number has been
+ set.
+ :param instrument: The instrument the data is from.
+ :param sample_path: The path to the data file that was focused.
+ :param bank: The name of the bank being saved.
+ :param sample_workspace: The name of the workspace to be saved.
+ :param rb_number: Usually an experiment id, defines the name of the user directory.
+ """
nexus_output_path = path.join(
path_handling.OUT_FILES_ROOT_DIR, "Focus",
- self.generate_output_file_name(instrument, sample_path, bank, ".nxs"))
+ self._generate_output_file_name(instrument, sample_path, bank, ".nxs"))
SaveNexus(InputWorkspace=sample_workspace, Filename=nexus_output_path)
if rb_number is not None:
nexus_output_path = path.join(
path_handling.OUT_FILES_ROOT_DIR, "User", rb_number, "Focus",
- self.generate_output_file_name(instrument, sample_path, bank, ".nxs"))
+ self._generate_output_file_name(instrument, sample_path, bank, ".nxs"))
SaveNexus(InputWorkspace=sample_workspace, Filename=nexus_output_path)
@staticmethod
- def generate_output_file_name(instrument, sample_path, bank, suffix):
+ def _generate_output_file_name(instrument, sample_path, bank, suffix):
run_no = path_handling.get_run_number_from_path(sample_path, instrument)
return instrument + "_" + run_no + "_bank_" + bank + suffix
diff --git a/scripts/Engineering/gui/engineering_diffraction/tabs/focus/presenter.py b/scripts/Engineering/gui/engineering_diffraction/tabs/focus/presenter.py
index 59989643cb0..eef9a62faee 100644
--- a/scripts/Engineering/gui/engineering_diffraction/tabs/focus/presenter.py
+++ b/scripts/Engineering/gui/engineering_diffraction/tabs/focus/presenter.py
@@ -30,18 +30,26 @@ class FocusPresenter(object):
self.rb_num = None
def on_focus_clicked(self):
- banks = self.get_banks()
- if not self.validate(banks):
+ banks = self._get_banks()
+ if not self._validate(banks):
return
focus_path = self.view.get_focus_filename()
self.start_focus_worker(focus_path, banks, self.view.get_plot_output(), self.rb_num)
def start_focus_worker(self, focus_path, banks, plot_output, rb_num):
- self.worker = AsyncTask(self.model.focus_run,
- (focus_path, banks, plot_output, self.instrument, rb_num, self.current_calibration),
- error_cb=self._on_worker_error,
- finished_cb=self.enable_focus_controls)
- self.disable_focus_controls()
+ """
+ Focus data in a separate thread to stop the main GUI from hanging.
+ :param focus_path: The path to the file containing the data to focus.
+ :param banks: A list of banks that are to be focused.
+ :param plot_output: True if the output should be plotted.
+ :param rb_num: The rb_number from the main window (often an experiment id)
+ """
+ self.worker = AsyncTask(
+ self.model.focus_run,
+ (focus_path, banks, plot_output, self.instrument, rb_num, self.current_calibration),
+ error_cb=self._on_worker_error,
+ finished_cb=self._enable_focus_controls)
+ self._disable_focus_controls()
self.worker.start()
def set_instrument_override(self, instrument):
@@ -57,35 +65,41 @@ class FocusPresenter(object):
def set_rb_number(self, rb_number):
self.rb_num = rb_number
- def validate(self, banks):
+ def _validate(self, banks):
+ """
+ Ensure that the worker is ready to be started.
+ :param banks: A list of banks to focus.
+ :return: True if the worker can be started safely.
+ """
if not self.view.get_focus_valid():
return False
if self.current_calibration["vanadium_path"] is None:
- self.create_error_message("Load a calibration from the Calibration tab before focusing.")
+ self._create_error_message(
+ "Load a calibration from the Calibration tab before focusing.")
return False
if self.view.is_searching():
return False
if len(banks) == 0:
- self.create_error_message("Please select at least one bank.")
+ self._create_error_message("Please select at least one bank.")
return False
return True
- def create_error_message(self, message):
+ def _create_error_message(self, message):
QMessageBox.warning(self.view, "Engineering Diffraction - Error", str(message))
def _on_worker_error(self, failure_info):
logger.warning(str(failure_info))
- self.enable_focus_controls()
+ self._enable_focus_controls()
- def enable_focus_controls(self):
+ def _enable_focus_controls(self):
self.view.set_focus_button_enabled(True)
self.view.set_plot_output_enabled(True)
- def disable_focus_controls(self):
+ def _disable_focus_controls(self):
self.view.set_focus_button_enabled(False)
self.view.set_plot_output_enabled(False)
- def get_banks(self):
+ def _get_banks(self):
banks = []
if self.view.get_north_bank():
banks.append("North")
@@ -94,6 +108,10 @@ class FocusPresenter(object):
return banks
def update_calibration(self, calibration):
+ """
+ Update the current calibration following an call from a CalibrationNotifier
+ :param calibration: The new current calibration.
+ """
self.current_calibration = calibration
# -----------------------
diff --git a/scripts/Engineering/gui/engineering_diffraction/tabs/focus/test/test_focus_model.py b/scripts/Engineering/gui/engineering_diffraction/tabs/focus/test/test_focus_model.py
index ff6d0055b42..aa8174464a1 100644
--- a/scripts/Engineering/gui/engineering_diffraction/tabs/focus/test/test_focus_model.py
+++ b/scripts/Engineering/gui/engineering_diffraction/tabs/focus/test/test_focus_model.py
@@ -39,7 +39,7 @@ class FocusModelTest(unittest.TestCase):
self.model.focus_run("305761", ["1", "2"], False, "ENGINX", "0", blank_calibration)
self.assertEqual(fetch_van.call_count, 0)
- @patch(file_path + ".FocusModel.save_focused_output_files_as_nexus")
+ @patch(file_path + ".FocusModel._save_focused_output_files_as_nexus")
@patch(file_path + ".FocusModel._run_focus")
@patch(file_path + ".FocusModel._load_focus_sample_run")
@patch(file_path + ".vanadium_corrections.fetch_correction_workspaces")
@@ -54,7 +54,7 @@ class FocusModelTest(unittest.TestCase):
model.FOCUSED_OUTPUT_WORKSPACE_NAME + banks[-1],
"mocked_integ", "mocked_curves", banks[-1])
- @patch(file_path + ".FocusModel.save_focused_output_files_as_nexus")
+ @patch(file_path + ".FocusModel._save_focused_output_files_as_nexus")
@patch(file_path + ".FocusModel._plot_focused_workspace")
@patch(file_path + ".FocusModel._run_focus")
@patch(file_path + ".FocusModel._load_focus_sample_run")
@@ -67,7 +67,7 @@ class FocusModelTest(unittest.TestCase):
self.model.focus_run("305761", banks, True, "ENGINX", "0", self.current_calibration)
self.assertEqual(len(banks), plot_focus.call_count)
- @patch(file_path + ".FocusModel.save_focused_output_files_as_nexus")
+ @patch(file_path + ".FocusModel._save_focused_output_files_as_nexus")
@patch(file_path + ".FocusModel._plot_focused_workspace")
@patch(file_path + ".FocusModel._run_focus")
@patch(file_path + ".FocusModel._load_focus_sample_run")
@@ -86,17 +86,17 @@ class FocusModelTest(unittest.TestCase):
mocked_workspace = "mocked-workspace"
output_file = path.join(path_handling.OUT_FILES_ROOT_DIR, "Focus",
"ENGINX_123_bank_North.nxs")
- self.model.save_focused_output_files_as_nexus("ENGINX", "Path/To/ENGINX000123.whatever",
- "North", mocked_workspace, None)
+ self.model._save_focused_output_files_as_nexus("ENGINX", "Path/To/ENGINX000123.whatever",
+ "North", mocked_workspace, None)
self.assertEqual(1, save.call_count)
save.assert_called_with(Filename=output_file, InputWorkspace=mocked_workspace)
@patch(file_path + ".SaveNexus")
def test_save_output_files_nexus_with_RB_number(self, save):
- self.model.save_focused_output_files_as_nexus("ENGINX", "Path/To/ENGINX000123.whatever",
- "North", "mocked-workspace",
- "An Experiment Number")
+ self.model._save_focused_output_files_as_nexus("ENGINX", "Path/To/ENGINX000123.whatever",
+ "North", "mocked-workspace",
+ "An Experiment Number")
self.assertEqual(2, save.call_count)
diff --git a/scripts/Engineering/gui/engineering_diffraction/tabs/focus/test/test_focus_presenter.py b/scripts/Engineering/gui/engineering_diffraction/tabs/focus/test/test_focus_presenter.py
index 67b7a18d080..0ce6745477c 100644
--- a/scripts/Engineering/gui/engineering_diffraction/tabs/focus/test/test_focus_presenter.py
+++ b/scripts/Engineering/gui/engineering_diffraction/tabs/focus/test/test_focus_presenter.py
@@ -24,7 +24,10 @@ class FocusPresenterTest(unittest.TestCase):
@patch(tab_path + ".presenter.FocusPresenter.start_focus_worker")
def test_worker_started_with_correct_params(self, worker):
- self.presenter.current_calibration = {"vanadium_path": "Fake/Path", "ceria_path": "Fake/Path"}
+ self.presenter.current_calibration = {
+ "vanadium_path": "Fake/Path",
+ "ceria_path": "Fake/Path"
+ }
self.view.get_focus_filename.return_value = "305738"
self.view.get_north_bank.return_value = False
self.view.get_south_bank.return_value = True
@@ -34,7 +37,7 @@ class FocusPresenterTest(unittest.TestCase):
self.presenter.on_focus_clicked()
worker.assert_called_with("305738", ["South"], True, None)
- @patch(tab_path + ".presenter.FocusPresenter.validate")
+ @patch(tab_path + ".presenter.FocusPresenter._validate")
@patch(tab_path + ".presenter.FocusPresenter.start_focus_worker")
def test_worker_not_started_validate_fails(self, worker, valid):
valid.return_value = False
@@ -43,13 +46,13 @@ class FocusPresenterTest(unittest.TestCase):
worker.assert_not_called()
def test_controls_disabled_disables_both(self):
- self.presenter.disable_focus_controls()
+ self.presenter._disable_focus_controls()
self.view.set_focus_button_enabled.assert_called_with(False)
self.view.set_plot_output_enabled.assert_called_with(False)
def test_controls_enabled_enables_both(self):
- self.presenter.enable_focus_controls()
+ self.presenter._enable_focus_controls()
self.view.set_focus_button_enabled.assert_called_with(True)
self.view.set_plot_output_enabled.assert_called_with(True)
@@ -68,53 +71,60 @@ class FocusPresenterTest(unittest.TestCase):
self.view.get_north_bank.return_value = True
self.view.get_south_bank.return_value = True
- self.assertEqual(["North", "South"], self.presenter.get_banks())
+ self.assertEqual(["North", "South"], self.presenter._get_banks())
self.view.get_north_bank.return_value = True
self.view.get_south_bank.return_value = False
- self.assertEqual(["North"], self.presenter.get_banks())
+ self.assertEqual(["North"], self.presenter._get_banks())
self.view.get_north_bank.return_value = False
self.view.get_south_bank.return_value = True
- self.assertEqual(["South"], self.presenter.get_banks())
+ self.assertEqual(["South"], self.presenter._get_banks())
self.view.get_north_bank.return_value = False
self.view.get_south_bank.return_value = False
- self.assertEqual([], self.presenter.get_banks())
+ self.assertEqual([], self.presenter._get_banks())
def test_validate_with_invalid_focus_path(self):
self.view.get_focus_valid.return_value = False
banks = ["North", "South"]
- self.assertFalse(self.presenter.validate(banks))
+ self.assertFalse(self.presenter._validate(banks))
- @patch(tab_path + ".presenter.FocusPresenter.create_error_message")
+ @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}
banks = ["North", "South"]
- self.presenter.validate(banks)
- create_error.assert_called_with("Load a calibration from the Calibration tab before focusing.")
+ self.presenter._validate(banks)
+ create_error.assert_called_with(
+ "Load a calibration from the Calibration tab before focusing.")
- @patch(tab_path + ".presenter.FocusPresenter.create_error_message")
+ @patch(tab_path + ".presenter.FocusPresenter._create_error_message")
def test_validate_while_searching(self, create_error):
- self.presenter.current_calibration = {"vanadium_path": "Fake/File/Path", "ceria_path": "Fake/Path"}
+ self.presenter.current_calibration = {
+ "vanadium_path": "Fake/File/Path",
+ "ceria_path": "Fake/Path"
+ }
self.view.is_searching.return_value = True
banks = ["North", "South"]
- self.assertEqual(False, self.presenter.validate(banks))
+ self.assertEqual(False, self.presenter._validate(banks))
self.assertEqual(0, create_error.call_count)
- @patch(tab_path + ".presenter.FocusPresenter.create_error_message")
+ @patch(tab_path + ".presenter.FocusPresenter._create_error_message")
def test_validate_with_no_banks_selected(self, create_error):
- self.presenter.current_calibration = {"vanadium_path": "Fake/Path", "ceria_path": "Fake/Path"}
+ self.presenter.current_calibration = {
+ "vanadium_path": "Fake/Path",
+ "ceria_path": "Fake/Path"
+ }
self.view.is_searching.return_value = False
banks = []
- self.presenter.validate(banks)
+ self.presenter._validate(banks)
create_error.assert_called_with("Please select at least one bank.")
--
GitLab