Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
alexhroom committed Oct 3, 2024
1 parent 13952cb commit 13e6ab6
Show file tree
Hide file tree
Showing 9 changed files with 100 additions and 35 deletions.
1 change: 1 addition & 0 deletions rascal2/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
RASCAL2_VERSION = "0.0.0"
19 changes: 10 additions & 9 deletions rascal2/core/runner.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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 = []
Expand All @@ -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()

Expand All @@ -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(
Expand All @@ -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


Expand Down
5 changes: 3 additions & 2 deletions rascal2/core/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
16 changes: 16 additions & 0 deletions rascal2/ui/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 9 additions & 4 deletions rascal2/ui/presenter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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):
Expand Down
1 change: 0 additions & 1 deletion rascal2/ui/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
39 changes: 27 additions & 12 deletions rascal2/widgets/terminal.py
Original file line number Diff line number Diff line change
@@ -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."""
Expand Down Expand Up @@ -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"<b>RasCAL-2:</b> software for neutron reflectivity calculations <b>v{version}</b>")
self.write_html(f"<b>RasCAL-2:</b> software for neutron reflectivity calculations <b>v{RASCAL2_VERSION}</b>")

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):
Expand Down
38 changes: 32 additions & 6 deletions tests/core/test_runner.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -50,31 +50,37 @@ 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!"],
],
)
@patch("rascal2.core.runner.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")
Expand Down Expand Up @@ -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:
Expand All @@ -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!"
3 changes: 2 additions & 1 deletion tests/test_presenter.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def __init__(self):
self.handle_results = MagicMock()
self.end_run = MagicMock()
self.logging = MagicMock()
self.settings = MagicMock()


@pytest.fixture
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 13e6ab6

Please sign in to comment.