From 13e6ab60ae539f36dd2706ce8c0c21c0da51c442 Mon Sep 17 00:00:00 2001 From: alexhroom Date: Tue, 1 Oct 2024 14:00:33 +0100 Subject: [PATCH] review fixes --- rascal2/__init__.py | 1 + rascal2/core/runner.py | 19 +++++++++--------- rascal2/core/settings.py | 5 +++-- rascal2/ui/model.py | 16 +++++++++++++++ rascal2/ui/presenter.py | 13 +++++++++---- rascal2/ui/view.py | 1 - rascal2/widgets/terminal.py | 39 +++++++++++++++++++++++++------------ tests/core/test_runner.py | 38 ++++++++++++++++++++++++++++++------ tests/test_presenter.py | 3 ++- 9 files changed, 100 insertions(+), 35 deletions(-) diff --git a/rascal2/__init__.py b/rascal2/__init__.py index e69de29..91a1cf5 100644 --- a/rascal2/__init__.py +++ b/rascal2/__init__.py @@ -0,0 +1 @@ +RASCAL2_VERSION = "0.0.0" diff --git a/rascal2/core/runner.py b/rascal2/core/runner.py index 631e55b..a9d882f 100644 --- a/rascal2/core/runner.py +++ b/rascal2/core/runner.py @@ -1,6 +1,7 @@ """QObject for running RAT.""" from dataclasses import dataclass +from logging import INFO from multiprocessing import Process, Queue import RATapi as RAT @@ -26,6 +27,7 @@ def __init__(self, rat_inputs, procedure: Procedures, display_on: bool): self.process = Process(target=run, args=(self.queue, rat_inputs, procedure, display_on)) + self.updated_problem = None self.results = None self.error = None self.events = [] @@ -46,17 +48,16 @@ def check_queue(self): if not self.process.is_alive(): self.timer.stop() self.queue.put(None) - # the queue should never be empty at this line (because we just put a - # None in it) but if it is, it'd hang forever - we add the timeout - # to avoid this happening just in case - for item in iter(lambda: self.queue.get(timeout=500), None): - if isinstance(item, (RAT.outputs.Results, RAT.outputs.BayesResults)): - self.results = item + for item in iter(self.queue.get, None): + if isinstance(item, tuple): + self.updated_problem, self.results = item self.finished.emit() elif isinstance(item, Exception): self.error = item self.stopped.emit() else: # else, assume item is an event + if isinstance(item, str): + item.rstrip() self.events.append(item) self.event_received.emit() @@ -81,7 +82,7 @@ def run(queue, rat_inputs: tuple, procedure: str, display: bool): if display: RAT.events.register(RAT.events.EventTypes.Message, queue.put) RAT.events.register(RAT.events.EventTypes.Progress, queue.put) - queue.put(LogData(20, "Starting RAT")) + queue.put(LogData(INFO, "Starting RAT")) try: problem_definition, output_results, bayes_results = RAT.rat_core.RATMain( @@ -93,10 +94,10 @@ def run(queue, rat_inputs: tuple, procedure: str, display: bool): return if display: - queue.put(LogData(20, "Finished RAT")) + queue.put(LogData(INFO, "Finished RAT")) RAT.events.clear() - queue.put(results) + queue.put((problem_definition, results)) return diff --git a/rascal2/core/settings.py b/rascal2/core/settings.py index 45cb7f0..ec7c1bd 100644 --- a/rascal2/core/settings.py +++ b/rascal2/core/settings.py @@ -107,8 +107,9 @@ class Settings(BaseModel, validate_assignment=True, arbitrary_types_allowed=True editor_fontsize: int = Field(default=12, title=SettingsGroups.General, description="Editor Font Size", gt=0) terminal_fontsize: int = Field(default=12, title=SettingsGroups.General, description="Terminal Font Size", gt=0) - log_path: str = Field(default="logs/rascal.log", title=SettingsGroups.Logging, description="Path to Log File") - log_level: LogLevels = Field(default=LogLevels.Info, title=SettingsGroups.Logging, description="Minimum Log Level") + log_path: str = Field(default="logs/rascal.log", title="logging", description="Path to Log File") + log_level: LogLevels = Field(default=LogLevels.Info, title="logging", description="Minimum Log Level") + clear_terminal: bool = Field(default=True, title="logging", description="Clear Terminal when Run Starts") mdi_defaults: MDIGeometries = Field( default=None, title=SettingsGroups.Windows, description="Default Window Geometries" diff --git a/rascal2/ui/model.py b/rascal2/ui/model.py index 4e533dd..56ee1c1 100644 --- a/rascal2/ui/model.py +++ b/rascal2/ui/model.py @@ -27,3 +27,19 @@ def create_project(self, name: str, save_path: str): self.project = RAT.Project(name=name) self.controls = RAT.Controls() self.save_path = save_path + + def update_project(self, problem_definition: RAT.rat_core.ProblemDefinition): + """Update the project given a set of results.""" + parameter_field = { + "parameters": "params", + "bulk_in": "bulkIn", + "bulk_out": "bulkOut", + "scalefactors": "scalefactors", + "domain_ratios": "domainRatio", + "background_parameters": "backgroundParams", + "resolution_parameters": "resolutionParams", + } + + for class_list in RAT.project.parameter_class_lists: + for index, value in enumerate(getattr(problem_definition, parameter_field[class_list])): + getattr(self.project, class_list)[index].value = value diff --git a/rascal2/ui/presenter.py b/rascal2/ui/presenter.py index 198cac3..35c9bc2 100644 --- a/rascal2/ui/presenter.py +++ b/rascal2/ui/presenter.py @@ -75,11 +75,16 @@ def edit_controls(self, setting: str, value: Any): return True def interrupt_terminal(self): - """Sends an interrupt signal to the terminal.""" + """Sends an interrupt signal to the RAT runner.""" self.runner.interrupt() def run(self): """Run RAT.""" + # reset terminal + self.view.terminal_widget.progress_bar.setVisible(False) + if self.view.settings.clear_terminal: + self.view.terminal_widget.clear() + rat_inputs = RAT.inputs.make_input(self.model.project, self.model.controls) display_on = self.model.controls.display != RAT.utils.enums.Display.Off @@ -91,15 +96,15 @@ def run(self): def handle_results(self): """Handle a RAT run being finished.""" - results = self.runner.results - self.view.handle_results(results) + self.model.update_project(self.runner.updated_problem) + self.view.handle_results(self.runner.results) def handle_interrupt(self): """Handle a RAT run being interrupted.""" if self.runner.error is None: self.view.logging.info("RAT run interrupted!") else: - self.view.logging.critical(f"RAT run failed with exception:\n{self.runner.error}") + self.view.logging.error(f"RAT run failed with exception:\n{self.runner.error}") self.view.end_run() def handle_event(self): diff --git a/rascal2/ui/view.py b/rascal2/ui/view.py index aa838cc..8fbacdf 100644 --- a/rascal2/ui/view.py +++ b/rascal2/ui/view.py @@ -307,4 +307,3 @@ def handle_results(self, results): def end_run(self): """Reset widgets after a run.""" self.controls_widget.run_button.setChecked(False) - self.terminal_widget.progress_bar.setVisible(False) diff --git a/rascal2/widgets/terminal.py b/rascal2/widgets/terminal.py index 91a3f07..a9a88ff 100644 --- a/rascal2/widgets/terminal.py +++ b/rascal2/widgets/terminal.py @@ -1,10 +1,11 @@ """Widget for terminal display.""" import logging -from importlib import metadata from PyQt6 import QtGui, QtWidgets +from rascal2 import RASCAL2_VERSION + class TerminalWidget(QtWidgets.QWidget): """Widget for displaying program output.""" @@ -40,26 +41,40 @@ def __init__(self, parent=None): widget_layout.setContentsMargins(0, 0, 0, 0) self.setLayout(widget_layout) - version = metadata.version("rascal2") self.write( """ - ███████████ █████████ █████████ █████ -░░███░░░░░███ ███░░░░░███ ███░░░░░███ ░░███ - ░███ ░███ ██████ █████ ███ ░░░ ░███ ░███ ░███ - ░██████████ ░░░░░███ ███░░ ░███ ░███████████ ░███ - ░███░░░░░███ ███████ ░░█████ ░███ ░███░░░░░███ ░███ + ███████████ █████████ █████████ █████ +░░███░░░░░███ ███░░░░░███ ███░░░░░███ ░░███ + ░███ ░███ ██████ █████ ███ ░░░ ░███ ░███ ░███ + ░██████████ ░░░░░███ ███░░ ░███ ░███████████ ░███ + ░███░░░░░███ ███████ ░░█████ ░███ ░███░░░░░███ ░███ ░███ ░███ ███░░███ ░░░░███░░███ ███ ░███ ░███ ░███ █ █████ █████░░████████ ██████ ░░█████████ █████ █████ ███████████ -░░░░░ ░░░░░ ░░░░░░░░ ░░░░░░ ░░░░░░░░░ ░░░░░ ░░░░░ ░░░░░░░░░░░ +░░░░░ ░░░░░ ░░░░░░░░ ░░░░░░ ░░░░░░░░░ ░░░░░ ░░░░░ ░░░░░░░░░░░ """ ) - self.write_html(f"RasCAL-2: software for neutron reflectivity calculations v{version}") + self.write_html(f"RasCAL-2: software for neutron reflectivity calculations v{RASCAL2_VERSION}") + + def write(self, text: str): + """Append plain text to the terminal. + + Parameters + ---------- + text : str + The text to append. - def write(self, text): + """ self.text_area.appendPlainText(text) - def write_html(self, text): - """Append HTML text to the terminal.""" + def write_html(self, text: str): + """Append HTML text to the terminal. + + Parameters + ---------- + text : str + The HTML to append. + + """ self.text_area.appendHtml(text) def clear(self): diff --git a/tests/core/test_runner.py b/tests/core/test_runner.py index d06657e..78c7c6d 100644 --- a/tests/core/test_runner.py +++ b/tests/core/test_runner.py @@ -1,6 +1,6 @@ """Tests for the RATRunner class.""" -from multiprocessing import Queue +from queue import Queue # we need a non-multiprocessing queue because mocks cannot be serialised from unittest.mock import MagicMock, patch import pytest @@ -50,9 +50,10 @@ def test_interrupt(mock_process): "queue_items", [ ["message!"], - ["message!", MagicMock(spec=RAT.outputs.Results)], - [MagicMock(spec=RAT.outputs.BayesResults)], + ["message!", (MagicMock(spec=RAT.rat_core.ProblemDefinition), MagicMock(spec=RAT.outputs.Results))], + [(MagicMock(spec=RAT.rat_core.ProblemDefinition), MagicMock(spec=RAT.outputs.BayesResults))], [make_progress_event(0.6)], + [make_progress_event(0.5), ValueError("Runner error!")], ["message 1!", make_progress_event(0.4), "message 2!"], ], ) @@ -60,21 +61,26 @@ def test_interrupt(mock_process): def test_check_queue(mock_process, queue_items): """Test that queue data is appropriately assigned.""" runner = RATRunner([], "", True) + runner.queue = Queue() for item in queue_items: runner.queue.put(item) runner.check_queue() - assert len(runner.events) == len([x for x in queue_items if not isinstance(x, RAT.outputs.Results)]) + assert len(runner.events) == len([x for x in queue_items if not isinstance(x, (tuple, Exception))]) for i, item in enumerate(runner.events): if isinstance(item, RAT.events.ProgressEventData): assert item.percent == queue_items[i].percent else: assert item == queue_items[i] - if runner.results is not None: + if isinstance(queue_items[-1], tuple): + assert isinstance(runner.updated_problem, RAT.rat_core.ProblemDefinition) assert isinstance(runner.results, RAT.outputs.Results) + if isinstance(queue_items[-1], Exception): + assert isinstance(runner.error, ValueError) + assert str(runner.error) == "Runner error!" @patch("rascal2.core.runner.Process") @@ -106,7 +112,7 @@ def test_run(display): while not queue.empty(): item = queue.get() - if isinstance(item, RAT.outputs.Results): + if isinstance(item, tuple): # ensure results were the last item to be added assert queue.empty() else: @@ -115,3 +121,23 @@ def test_run(display): assert item.percent == expected_item else: assert item == expected_item + + +def test_run_error(): + """If RATMain produces an error, it should be added to the queue.""" + + def erroring_ratmain(*args): + """A RATMain mock that raises an error.""" + raise ValueError("RAT Main Error!") + + queue = Queue() + with patch("RATapi.rat_core.RATMain", new=erroring_ratmain): + run(queue, [0, 1, 2, 3, 4], "", True) + + queue.put(None) + queue_contents = list(iter(queue.get, None)) + assert len(queue_contents) == 2 + assert isinstance(queue_contents[0], LogData) + error = queue_contents[1] + assert isinstance(error, ValueError) + assert str(error) == "RAT Main Error!" diff --git a/tests/test_presenter.py b/tests/test_presenter.py index f21729d..22774ca 100644 --- a/tests/test_presenter.py +++ b/tests/test_presenter.py @@ -33,6 +33,7 @@ def __init__(self): self.handle_results = MagicMock() self.end_run = MagicMock() self.logging = MagicMock() + self.settings = MagicMock() @pytest.fixture @@ -101,7 +102,7 @@ def test_run_error(presenter): presenter.runner = MagicMock() presenter.runner.error = ValueError("Test error!") presenter.handle_interrupt() - presenter.view.logging.critical.assert_called_once_with("RAT run failed with exception:\nTest error!") + presenter.view.logging.error.assert_called_once_with("RAT run failed with exception:\nTest error!") @pytest.mark.parametrize(