From 828d3d9e99f6f6e15d9ece278d8365182a9be5fa Mon Sep 17 00:00:00 2001 From: Stephen Nneji Date: Wed, 14 Jan 2026 15:44:15 +0000 Subject: [PATCH 1/2] Makes all settings global --- rascal2/config.py | 83 +++++++-------------------- rascal2/dialogs/custom_file_editor.py | 6 +- rascal2/dialogs/settings_dialog.py | 8 +-- rascal2/main.py | 6 +- rascal2/settings.py | 21 ++++--- rascal2/ui/presenter.py | 14 ++--- rascal2/ui/view.py | 40 +++++-------- rascal2/widgets/plot.py | 13 ++++- rascal2/widgets/terminal.py | 28 +++++++++ tests/conftest.py | 35 ++++++++++- tests/core/test_settings.py | 41 ++++++++----- tests/test_config.py | 39 ++++++------- tests/test_ui.py | 4 +- tests/ui/test_presenter.py | 32 +++++------ tests/ui/test_view.py | 74 +++++++++++++----------- 15 files changed, 237 insertions(+), 207 deletions(-) diff --git a/rascal2/config.py b/rascal2/config.py index 77d66c13..51591f04 100644 --- a/rascal2/config.py +++ b/rascal2/config.py @@ -8,7 +8,7 @@ import site import sys -from rascal2.settings import Settings, get_global_settings +from rascal2.settings import get_global_settings if getattr(sys, "frozen", False): # we are running in a bundle @@ -50,84 +50,41 @@ def path_for(filename: str): return (IMAGES_PATH / filename).as_posix() -def setup_settings(project_path: str | os.PathLike) -> Settings: - """Set up the Settings object for the project. - - Parameters - ---------- - project_path : str or PathLike - The path to the current RasCAL-2 project. - - Returns - ------- - Settings - If a settings.json file already exists in the - RasCAL-2 project, returns a Settings object with - the settings defined there. Otherwise, returns a - (global) default Settings object. - - """ - filepath: pathlib.Path = pathlib.Path(project_path, "settings.json") - if filepath.is_file(): - json = filepath.read_text() - return Settings.model_validate_json(json) - return Settings() +def log_uncaught_exceptions(exc_type, exc_value, exc_traceback): + """Qt slots swallows exceptions but this ensures exceptions are logged.""" + logging.critical("An unhandled exception occurred!", exc_info=(exc_type, exc_value, exc_traceback)) + logging.shutdown() + sys.exit(1) -def setup_logging(log_path: str | os.PathLike, terminal=None, level: int = logging.INFO) -> logging.Logger: +def setup_logging(level: int = logging.INFO) -> None: """Set up logging for the project. - The default logging path and level are defined in the settings. - Parameters ---------- - log_path : str | PathLike - The path to where the log file will be written. - terminal : Optional[TerminalWidget] - The TerminalWidget instance which acts as an IO stream. level : int, default logging.INFO The debug level for the logger. """ - path = pathlib.Path(log_path) - logger = logging.getLogger("rascal_log") + path = pathlib.Path(get_global_settings().fileName()).parent + path.mkdir(parents=True, exist_ok=True) + + logger = logging.getLogger() logger.setLevel(level) logger.handlers.clear() - log_filehandler = logging.FileHandler(path) + log_file_handler = logging.FileHandler(path / "rascal.log") file_formatting = logging.Formatter("%(asctime)s - %(threadName)s - %(name)s - %(levelname)s - %(message)s") - log_filehandler.setFormatter(file_formatting) - logger.addHandler(log_filehandler) - - if terminal is not None: - # handler that logs to terminal widget - log_termhandler = logging.StreamHandler(stream=terminal) - term_formatting = logging.Formatter("%(levelname)s - %(message)s") - log_termhandler.setFormatter(term_formatting) - logger.addHandler(log_termhandler) - - return logger - - -def get_logger(): - """Get the RasCAL logger, and set up a backup logger if it hasn't been set up yet.""" - logger = logging.getLogger("rascal_log") - if not logger.handlers: - # Backup in case the crash happens before the local logger setup - path = pathlib.Path(get_global_settings().fileName()).parent - path.mkdir(parents=True, exist_ok=True) - setup_logging(path / "rascal.log") + log_file_handler.setFormatter(file_formatting) + logger.addHandler(log_file_handler) - return logger + # handler that logs to terminal widget + log_term_handler = logging.StreamHandler() + term_formatting = logging.Formatter("%(levelname)s - %(message)s") + log_term_handler.setFormatter(term_formatting) + logger.addHandler(log_term_handler) - -def log_uncaught_exceptions(exc_type, exc_value, exc_traceback): - """Qt slots swallows exceptions but this ensures exceptions are logged.""" - logger = get_logger() - logger.addHandler(logging.StreamHandler(stream=sys.stderr)) # print emergency crashes to terminal - logger.critical("An unhandled exception occurred!", exc_info=(exc_type, exc_value, exc_traceback)) - logging.shutdown() - sys.exit(1) + sys.excepthook = log_uncaught_exceptions def run_matlab(ready_event, close_event, engine_output): diff --git a/rascal2/dialogs/custom_file_editor.py b/rascal2/dialogs/custom_file_editor.py index ad1f243e..f904eae7 100644 --- a/rascal2/dialogs/custom_file_editor.py +++ b/rascal2/dialogs/custom_file_editor.py @@ -37,8 +37,7 @@ def edit_file_matlab(filename: str): try: engine = MatlabHelper().get_local_engine() except Exception as ex: - logger = logging.getLogger("rascal_log") - logger.error("Attempted to edit a file in MATLAB engine" + repr(ex)) + logging.error("Attempted to edit a file in MATLAB engine", exc_info=ex) return engine.edit(str(filename)) @@ -131,7 +130,6 @@ def save_file(self): self.file.write_text(self.editor.text()) self.accept() except OSError as ex: - logger = logging.getLogger("rascal_log") message = f"Failed to save custom file to {self.file}.\n" - logger.error(message, exc_info=ex) + logging.error(message, exc_info=ex) QtWidgets.QMessageBox.critical(self, "Save File", message, QtWidgets.QMessageBox.StandardButton.Ok) diff --git a/rascal2/dialogs/settings_dialog.py b/rascal2/dialogs/settings_dialog.py index 0ed71d19..5abc4a0a 100644 --- a/rascal2/dialogs/settings_dialog.py +++ b/rascal2/dialogs/settings_dialog.py @@ -6,7 +6,7 @@ from PyQt6 import QtCore, QtWidgets from rascal2.config import MATLAB_ARCH_FILE, MatlabHelper -from rascal2.settings import Settings, SettingsGroups, delete_local_settings +from rascal2.settings import SettingsGroups from rascal2.widgets.inputs import get_validated_input @@ -61,15 +61,13 @@ def __init__(self, parent): def update_settings(self) -> None: """Accept the changed settings.""" self.parent().settings = self.settings - if self.parent().presenter.model.save_path: - self.parent().settings.save(self.parent().presenter.model.save_path) + self.parent().settings.set_global_settings() self.matlab_tab.set_matlab_paths() self.accept() def reset_default_settings(self) -> None: """Reset the settings to the global defaults.""" - delete_local_settings(self.parent().presenter.model.save_path) - self.parent().settings = Settings() + self.parent().settings.reset_global_settings() self.accept() diff --git a/rascal2/main.py b/rascal2/main.py index a6b9155e..b02378c3 100644 --- a/rascal2/main.py +++ b/rascal2/main.py @@ -1,3 +1,4 @@ +import logging import multiprocessing import re import sys @@ -5,7 +6,7 @@ from PyQt6 import QtGui, QtWidgets -from rascal2.config import IMAGES_PATH, STATIC_PATH, MatlabHelper, handle_scaling, log_uncaught_exceptions, path_for +from rascal2.config import IMAGES_PATH, STATIC_PATH, MatlabHelper, handle_scaling, path_for, setup_logging from rascal2.ui.view import MainWindowView @@ -42,10 +43,11 @@ def main(): """Entry point function for starting RasCAL.""" multiprocessing.freeze_support() multiprocessing.set_start_method("spawn", force=True) - sys.excepthook = log_uncaught_exceptions + setup_logging() matlab_helper = MatlabHelper() exit_code = ui_execute() matlab_helper.close_event.set() + logging.shutdown() sys.exit(exit_code) diff --git a/rascal2/settings.py b/rascal2/settings.py index 699500f4..db6873fe 100644 --- a/rascal2/settings.py +++ b/rascal2/settings.py @@ -47,7 +47,6 @@ class SettingsGroups(StrEnum): Logging = "Logging" Plotting = "Plotting" Terminal = "Terminal" - Windows = "Windows" class Styles(StrEnum): @@ -131,10 +130,6 @@ class Settings(BaseModel, validate_assignment=True, arbitrary_types_allowed=True default=True, title=SettingsGroups.Terminal, description="Clear Terminal when Run Starts" ) terminal_fontsize: int = Field(default=12, title=SettingsGroups.Terminal, description="Terminal Font Size", gt=0) - - mdi_defaults: MDIGeometries = Field( - default=None, title=SettingsGroups.Windows, description="Default Window Geometries" - ) export_background_colour: BackgroundColour = Field( default=BackgroundColour.White, title=SettingsGroups.Plotting, description="Background colour of exported plot" ) @@ -145,7 +140,6 @@ def model_post_init(self, __context: Any): for setting in unset_settings: if global_name(setting) in global_settings.allKeys(): setattr(self, setting, global_settings.value(global_name(setting))) - self.model_fields_set.remove(setting) # we don't want global defaults to count as manually set! def save(self, path: str | PathLike): """Save settings to a JSON file in the given path. @@ -163,6 +157,17 @@ def set_global_settings(self): global_settings = get_global_settings() for setting in self.model_fields_set: global_settings.setValue(global_name(setting), getattr(self, setting)) + global_settings.sync() + + def reset_global_settings(self): + """Reset the local and global settings to default.""" + for field, field_info in self.model_fields.items(): + setattr(self, field, field_info.default) + + global_settings = get_global_settings() + for group in SettingsGroups: + global_settings.remove(group) + global_settings.sync() def global_name(key: str) -> str: @@ -202,7 +207,7 @@ def update_recent_projects(path: str | None = None) -> list[str]: """ settings = get_global_settings() - recent_projects: list[str] = settings.value("internal/recent_projects") + recent_projects: list[str] = settings.value("recent_projects") if not recent_projects: recent_projects = [] @@ -213,6 +218,6 @@ def update_recent_projects(path: str | None = None) -> list[str]: new_recent_projects = new_recent_projects[:10] - settings.setValue("internal/recent_projects", new_recent_projects) + settings.setValue("recent_projects", new_recent_projects) settings.sync() return new_recent_projects diff --git a/rascal2/ui/presenter.py b/rascal2/ui/presenter.py index 3da24bc1..3ddd2402 100644 --- a/rascal2/ui/presenter.py +++ b/rascal2/ui/presenter.py @@ -1,3 +1,4 @@ +import logging import re import warnings from typing import Any @@ -71,11 +72,10 @@ def load_r1_project(self, load_path: str): def initialise_ui(self): """Initialise UI for a project.""" - suffix = " [Example]" if self.model.is_project_example() else "" + suffix = " [Example]" if self.model.is_project_example() else f"[{self.model.save_path}]" self.view.setWindowTitle( self.view.windowTitle().split(" - ")[0] + " - " + self.model.project.name + suffix, ) - self.view.init_settings_and_log(self.model.save_path) self.view.setup_mdi() self.view.plot_widget.update_plots() self.view.handle_results(self.model.results) @@ -124,7 +124,7 @@ def save_project(self, save_as: bool = False): try: self.model.save_project(to_path) except OSError as err: - self.view.logging.error(f"Failed to save project to {to_path}.\n", exc_info=err) + logging.error(f"Failed to save project to {to_path}.\n", exc_info=err) else: update_recent_projects(self.model.save_path) self.view.undo_stack.setClean() @@ -158,7 +158,7 @@ def export_fits(self): try: write_result_to_zipped_csvs(save_file, results) except OSError as err: - self.view.logging.error(f"Failed to save fits to {save_file}.\n", exc_info=err) + logging.error(f"Failed to save fits to {save_file}.\n", exc_info=err) def interrupt_terminal(self): """Send an interrupt signal to the RAT runner.""" @@ -232,9 +232,9 @@ def handle_results(self): def handle_interrupt(self): """Handle a RAT run being interrupted.""" if self.runner.error is None: - self.view.logging.info("RAT run interrupted!") + logging.info("RAT run interrupted!") else: - self.view.logging.error("RAT run failed with exception.\n", exc_info=self.runner.error) + logging.error("RAT run failed with exception.\n", exc_info=self.runner.error) self.view.handle_results() self.model.controls.delete_IPC() @@ -252,7 +252,7 @@ def handle_event(self): case rat.events.PlotEventData(): self.view.plot_widget.plot_with_blit(event) case LogData(): - self.view.logging.log(event.level, event.msg) + logging.log(event.level, event.msg) def edit_project(self, updated_project: dict, preview: bool = True) -> None: """Edit the Project with a dictionary of attributes. diff --git a/rascal2/ui/view.py b/rascal2/ui/view.py index ffe45e17..ecc35faf 100644 --- a/rascal2/ui/view.py +++ b/rascal2/ui/view.py @@ -2,12 +2,12 @@ from PyQt6 import QtCore, QtGui, QtWidgets -from rascal2.config import EXAMPLES_PATH, EXAMPLES_TEMP_PATH, get_logger, path_for, setup_logging, setup_settings +from rascal2.config import EXAMPLES_PATH, EXAMPLES_TEMP_PATH, path_for from rascal2.core.enums import UnsavedReply from rascal2.dialogs.about_dialog import AboutDialog from rascal2.dialogs.settings_dialog import SettingsDialog from rascal2.dialogs.startup_dialog import PROJECT_FILES, LoadDialog, LoadR1Dialog, NewProjectDialog, StartupDialog -from rascal2.settings import MDIGeometries, Settings +from rascal2.settings import MDIGeometries, Settings, get_global_settings from rascal2.widgets import ControlsWidget, PlotWidget, TerminalWidget from rascal2.widgets.project import ProjectWidget from rascal2.widgets.startup import StartUpWidget @@ -51,11 +51,11 @@ def __init__(self): self.setAttribute(QtCore.Qt.WidgetAttribute.WA_DeleteOnClose) self.settings = Settings() - self.logging = get_logger() self.startup_dlg = StartUpWidget(self) self.setCentralWidget(self.startup_dlg) self.about_dialog = AboutDialog(self) + self.restoreGeometry(get_global_settings().value("window_geometry", bytearray(b""))) def closeEvent(self, event): if self.presenter.ask_to_save_project(): @@ -331,7 +331,11 @@ def setup_mdi_widgets(self): def reset_mdi_layout(self): """Reset MDI layout to the default.""" - if self.settings.mdi_defaults is None: + try: + mdi_defaults = get_global_settings().value("mdi_defaults") + except TypeError: + mdi_defaults = None + if mdi_defaults is None: for window in self.mdi.subWindowList(): window.showNormal() self.mdi.tileSubWindows() @@ -339,7 +343,7 @@ def reset_mdi_layout(self): for window in self.mdi.subWindowList(): # get corresponding MDIGeometries entry for the widget widget_name = window.windowTitle().lower().split(" ")[-1] - x, y, width, height, minimized = getattr(self.settings.mdi_defaults, widget_name) + x, y, width, height, minimized = getattr(mdi_defaults, widget_name) if minimized: window.showMinimized() else: @@ -355,23 +359,10 @@ def save_mdi_layout(self): geom = window.geometry() geoms[widget_name] = (geom.x(), geom.y(), geom.width(), geom.height(), window.isMinimized()) - self.settings.mdi_defaults = MDIGeometries.model_validate(geoms) - - def init_settings_and_log(self, save_path: str): - """Initialise settings and logging for the project. - - Parameters - ---------- - save_path : str - The save path for the project. - - """ - proj_path = Path(save_path) - self.settings = setup_settings(proj_path) - - log_path = proj_path / "logs" - log_path.mkdir(parents=True, exist_ok=True) - self.logging = setup_logging(log_path / "rascal.log", self.terminal_widget) + global_setting = get_global_settings() + global_setting.setValue("mdi_defaults", MDIGeometries.model_validate(geoms)) + global_setting.setValue("window_geometry", self.saveGeometry()) + global_setting.sync() def enable_elements(self): """Enable the elements that are disabled on startup.""" @@ -503,11 +494,8 @@ def show_confirm_stop_calculation_dialog(self) -> bool: no_show_check_box = QtWidgets.QCheckBox("Do not show this again") message_box.setCheckBox(no_show_check_box) no_show_check_box.toggled.connect(lambda val: setattr(self.settings, "show_stop_calculation_warning", not val)) - - # Make this save to general settings - message_box.exec() - + self.settings.set_global_settings() return message_box.clickedButton() == yes_button def show_unsaved_dialog(self, message: str) -> UnsavedReply: diff --git a/rascal2/widgets/plot.py b/rascal2/widgets/plot.py index 8945f478..ff9ba413 100644 --- a/rascal2/widgets/plot.py +++ b/rascal2/widgets/plot.py @@ -12,6 +12,17 @@ from rascal2.widgets.inputs import MultiSelectComboBox, ProgressButton +class CanvasQT(FigureCanvasQTAgg): + """Workaround so plot window can be started minimised.""" + + def showEvent(self, event): + # Calling showMinimised in reset_mdi_layout causes a crash because window + # handle can be None. + window = self.window().windowHandle() + if window is not None: + super().showEvent(event) + + class PlotWidget(QtWidgets.QWidget): """The MDI plot widget.""" @@ -212,7 +223,7 @@ def __init__(self, parent): self.blit_plot = None self.figure = self.make_figure() - self.canvas = FigureCanvasQTAgg( + self.canvas = CanvasQT( self.figure, ) self.figure.set_facecolor("none") diff --git a/rascal2/widgets/terminal.py b/rascal2/widgets/terminal.py index dc8843c3..71185824 100644 --- a/rascal2/widgets/terminal.py +++ b/rascal2/widgets/terminal.py @@ -1,10 +1,29 @@ """Widget for terminal display.""" +import logging + from PyQt6 import QtGui, QtWidgets from rascal2 import RASCAL2_VERSION +class CustomStreamHandler(logging.StreamHandler): + """Log stream handler to ensure messages are formatted properly in terminal.""" + + def emit(self, record): + try: + msg = self.format(record) + self.terminator + if record.levelno >= logging.ERROR: + self.stream.write_error(msg) + else: + self.stream.write(msg) + self.flush() + except RecursionError: + raise + except Exception: + self.handleError(record) + + class TerminalWidget(QtWidgets.QWidget): """Widget for displaying program output.""" @@ -33,6 +52,15 @@ def __init__(self): widget_layout.setContentsMargins(0, 0, 0, 0) self.setLayout(widget_layout) + self.add_stream_handler() + + def add_stream_handler(self): + """Add terminal as a stream for the log handler.""" + logger = logging.getLogger() + log_term_handler = CustomStreamHandler(stream=self) + term_formatting = logging.Formatter("%(levelname)s - %(message)s") + log_term_handler.setFormatter(term_formatting) + logger.addHandler(log_term_handler) def write(self, text: str): """Append plain text to the terminal. diff --git a/tests/conftest.py b/tests/conftest.py index fbe6364c..ff31b498 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,9 +1,40 @@ +import tempfile +from pathlib import Path +from unittest.mock import patch + import pytest -from PyQt6.QtWidgets import QApplication +from PyQt6 import QtCore, QtWidgets -APP = QApplication([]) +APP = QtWidgets.QApplication([]) +GLOBAL_SETTING = None @pytest.fixture def qt_application(): return APP + + +@pytest.fixture +def global_setting(): + return GLOBAL_SETTING + + +@pytest.fixture(scope="session", autouse=True) +def mock_setting(request): + global GLOBAL_SETTING + tmp_dir = tempfile.TemporaryDirectory(ignore_cleanup_errors=True) + ini_file = Path(tmp_dir.name) / "settings.ini" + GLOBAL_SETTING = QtCore.QSettings(str(ini_file), QtCore.QSettings.Format.IniFormat) + setting_patch = [] + for target in ["rascal2.ui.view.get_global_settings", "rascal2.settings.get_global_settings"]: + setting_patch.append(patch(target, return_value=GLOBAL_SETTING)) + setting_patch[-1].start() + + def teardown_mock_setting(): + global GLOBAL_SETTING + GLOBAL_SETTING = None + tmp_dir.cleanup() + for target in setting_patch: + target.stop() + + request.addfinalizer(teardown_mock_setting) diff --git a/tests/core/test_settings.py b/tests/core/test_settings.py index 78d2c381..1572651c 100644 --- a/tests/core/test_settings.py +++ b/tests/core/test_settings.py @@ -5,6 +5,7 @@ from unittest.mock import patch import pytest +from PyQt6.QtCore import QSettings from rascal2.settings import Settings, delete_local_settings, update_recent_projects @@ -72,22 +73,32 @@ def test_save(kwargs): assert settings == loaded_settings -@patch("rascal2.settings.QtCore.QSettings.setValue") -def test_set_global(mock): +@patch("rascal2.settings.get_global_settings") +def test_set_and_reset_global(mock_get_global): """Test that we can set manually-set project settings as global settings.""" - settings = Settings() - settings.set_global_settings() - assert not mock.called - - settings = Settings(editor_fontsize=9) - settings.set_global_settings() - mock.assert_called_once_with("General/editor_fontsize", 9) - - mock.reset_mock() - settings = Settings(editor_fontsize=18, terminal_fontsize=3) - settings.set_global_settings() - mock.assert_any_call("General/editor_fontsize", 18) - mock.assert_any_call("Terminal/terminal_fontsize", 3) + with tempfile.TemporaryDirectory() as tmp_dir: + ini_file = Path(tmp_dir) / "settings.ini" + global_setting = QSettings(str(ini_file), QSettings.Format.IniFormat) + mock_get_global.return_value = global_setting + settings = Settings() + settings.set_global_settings() + assert global_setting.allKeys() == [] + + settings = Settings(editor_fontsize=9) + settings.set_global_settings() + assert global_setting.value("General/editor_fontsize") == 9 + settings.reset_global_settings() + assert settings.editor_fontsize == 12 + assert global_setting.allKeys() == [] + + settings = Settings(editor_fontsize=18, terminal_fontsize=3) + settings.set_global_settings() + assert global_setting.value("General/editor_fontsize") == 18 + assert global_setting.value("Terminal/terminal_fontsize") == 3 + settings.reset_global_settings() + assert settings.editor_fontsize == 12 + assert settings.terminal_fontsize == 12 + assert global_setting.allKeys() == [] @pytest.mark.parametrize( diff --git a/tests/test_config.py b/tests/test_config.py index 04152344..40768b6d 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,34 +1,29 @@ """Tests for configuration utilities.""" +import logging import tempfile from logging import CRITICAL, INFO, WARNING from pathlib import Path -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest -from rascal2.config import setup_logging, setup_settings -from rascal2.settings import LogLevels, Settings - - -def test_setup_settings(): - """Test that settings are grabbed from JSON or made from scratch correctly.""" - sets = setup_settings(".") - assert sets == Settings() - with tempfile.TemporaryDirectory() as tmp: - settings1 = Settings(editor_fontsize=21, log_level=LogLevels.Critical) - settings1.save(tmp) - sets_from_json = setup_settings(tmp) - assert sets_from_json == settings1 +from rascal2.config import setup_logging @pytest.mark.parametrize("level", [INFO, WARNING, CRITICAL]) -def test_setup_logging(level): +@patch("rascal2.config.get_global_settings") +def test_setup_logging(mock_get_global, level): """Test that the logger is set up correctly.""" - tmp = tempfile.mkdtemp() - terminal = MagicMock() - log = setup_logging(Path(tmp, "rascal.log"), terminal, level) - assert Path(tmp, "rascal.log").is_file() - - assert log.level == level - assert log.hasHandlers() + with tempfile.TemporaryDirectory() as tmp_dir: + path = Path(tmp_dir) / "settings.ini" + global_setting = MagicMock() + global_setting.fileName.return_value = path + mock_get_global.return_value = global_setting + setup_logging(level) + assert Path(tmp_dir, "rascal.log").is_file() + + log = logging.getLogger() + assert log.level == level + assert log.hasHandlers() + logging.shutdown() diff --git a/tests/test_ui.py b/tests/test_ui.py index 5c27f046..ae90d9ea 100644 --- a/tests/test_ui.py +++ b/tests/test_ui.py @@ -10,12 +10,12 @@ def make_main_window(request): def make(): no_exceptions = True - def test_exception_hook(exc_type, exc_value, exc_traceback): + def exception_hook(exc_type, exc_value, exc_traceback): nonlocal no_exceptions no_exceptions = False sys.__excepthook__(exc_type, exc_value, exc_traceback) - sys.excepthook = test_exception_hook + sys.excepthook = exception_hook window = MainWindowView() diff --git a/tests/ui/test_presenter.py b/tests/ui/test_presenter.py index a272779e..2eca31e9 100644 --- a/tests/ui/test_presenter.py +++ b/tests/ui/test_presenter.py @@ -41,21 +41,22 @@ def __init__(self): self.terminal_widget = MagicMock() self.plot_widget = MagicMock() self.handle_results = MagicMock() - self.logging = MagicMock() self.settings = MagicMock() self.get_project_folder = lambda: "new path/" @pytest.fixture def presenter(): - pr = MainWindowPresenter(MockWindowView()) - pr.runner = MagicMock() - pr.model.controls = Controls() - pr.model.project = MagicMock() - pr.model.results = MagicMock() - pr.model.save_path = "some_path/" + with patch("rascal2.ui.presenter.logging", autospec=True) as mock_log: + pr = MainWindowPresenter(MockWindowView()) + pr.runner = MagicMock() + pr.model.controls = Controls() + pr.model.project = MagicMock() + pr.model.results = MagicMock() + pr.model.save_path = "some_path/" + pr.logging = mock_log - return pr + yield pr @pytest.mark.parametrize(["param", "value"], [("nSamples", 50), ("parallel", "contrasts")]) @@ -127,7 +128,7 @@ def test_stop_run(presenter): presenter.runner = MagicMock() presenter.runner.error = None presenter.handle_interrupt() - presenter.view.logging.info.assert_called_once_with("RAT run interrupted!") + presenter.logging.info.assert_called_once_with("RAT run interrupted!") def test_run_error(presenter): @@ -135,9 +136,7 @@ def test_run_error(presenter): presenter.runner = MagicMock() presenter.runner.error = ValueError("Test error!") presenter.handle_interrupt() - presenter.view.logging.error.assert_called_once_with( - "RAT run failed with exception.\n", exc_info=presenter.runner.error - ) + presenter.logging.error.assert_called_once_with("RAT run failed with exception.\n", exc_info=presenter.runner.error) @pytest.mark.parametrize( @@ -170,12 +169,11 @@ def test_handle_progress_event(presenter): def test_handle_log_data(presenter): presenter.runner.events = [LogData(10, "Test log!")] presenter.handle_event() - presenter.view.logging.log.assert_called_with(10, "Test log!") + presenter.logging.log.assert_called_with(10, "Test log!") @pytest.mark.parametrize("function", ["create_project", "load_project", "load_r1_project"]) -@patch("rascal2.ui.presenter.MainWindowModel", autospec=True) -def test_load_project(model_mock, presenter, function): +def test_load_project(presenter, function): """All the project initialisation functions should run the corresponding model function and initialise UI.""" end_function = MagicMock() setattr(presenter.model, function, MagicMock()) @@ -252,11 +250,11 @@ def test_export_fits(mock_write_result, presenter): error = OSError("Test Error") mock_write_result.side_effect = error presenter.view.get_save_file = MagicMock(return_value=test_zip_file) - presenter.view.logging.error = MagicMock() + presenter.logging.error = MagicMock() presenter.export_fits() mock_write_result.assert_called_once_with(test_zip_file, presenter.model.results) - presenter.view.logging.error.assert_called_once_with("Failed to save fits to test.zip.\n", exc_info=error) + presenter.logging.error.assert_called_once_with("Failed to save fits to test.zip.\n", exc_info=error) # If we do not have any results, don't ask for a file presenter.view.get_save_file.reset_mock() diff --git a/tests/ui/test_view.py b/tests/ui/test_view.py index 96a2c749..84e2e783 100644 --- a/tests/ui/test_view.py +++ b/tests/ui/test_view.py @@ -6,8 +6,9 @@ import pytest from PyQt6 import QtWidgets +from PyQt6.QtCore import QSettings -from rascal2.settings import MDIGeometries, Settings +from rascal2.settings import MDIGeometries from rascal2.ui.view import MainWindowView @@ -59,19 +60,20 @@ def test_view(): @patch("rascal2.ui.view.MainWindowPresenter") @patch("rascal2.ui.view.ControlsWidget.setup_controls") class TestMDISettings: - def test_reset_mdi(self, mock1, mock2, mock3, test_view, geometry): + def test_reset_mdi(self, mock1, mock2, mock3, global_setting, test_view, geometry): """Test that resetting the MDI works.""" - test_view.settings = Settings() test_view.setup_mdi() - test_view.settings.mdi_defaults = MDIGeometries( - plots=geometry[0], project=geometry[1], terminal=geometry[2], controls=geometry[3] + global_setting.setValue( + "mdi_defaults", + MDIGeometries(plots=geometry[0], project=geometry[1], terminal=geometry[2], controls=geometry[3]), ) test_view.reset_mdi_layout() + mdi_defaults = global_setting.value("mdi_defaults") for window in test_view.mdi.subWindowList(): # get corresponding MDIGeometries entry for the widget widget_name = window.windowTitle().lower().split(" ")[-1] w_geom = window.geometry() - assert getattr(test_view.settings.mdi_defaults, widget_name) == ( + assert getattr(mdi_defaults, widget_name) == ( w_geom.x(), w_geom.y(), w_geom.width(), @@ -79,9 +81,8 @@ def test_reset_mdi(self, mock1, mock2, mock3, test_view, geometry): window.isMinimized(), ) - def test_set_mdi(self, mock1, mock2, mock3, test_view, geometry): + def test_set_mdi(self, mock1, mock2, mock3, global_setting, test_view, geometry): """Test that setting the MDI adds the expected object to settings.""" - test_view.settings = Settings() test_view.setup_mdi() widgets_in_order = [] @@ -92,9 +93,10 @@ def test_set_mdi(self, mock1, mock2, mock3, test_view, geometry): window.showMinimized() test_view.save_mdi_layout() + mdi_defaults = global_setting.value("mdi_defaults") for i, widget in enumerate(widgets_in_order): window = test_view.mdi.subWindowList()[i] - assert getattr(test_view.settings.mdi_defaults, widget) == ( + assert getattr(mdi_defaults, widget) == ( window.x(), window.y(), window.width(), @@ -113,38 +115,44 @@ def test_set_enabled(test_view): @patch("PyQt6.QtWidgets.QFileDialog.getExistingDirectory") -def test_get_project_folder(mock_get_dir: MagicMock): +@patch("rascal2.ui.view.get_global_settings") +def test_get_project_folder(mock_get_global, mock_get_dir: MagicMock): """Test that getting a specified folder works as expected.""" - view = MainWindowView() - view.check_save_blacklist = MagicMock(return_value=False) - mock_overwrite = MagicMock(return_value=True) + with tempfile.TemporaryDirectory() as tmp_dir: + ini_file = Path(tmp_dir) / "settings.ini" + global_setting = QSettings(str(ini_file), QSettings.Format.IniFormat) + mock_get_global.return_value = global_setting - tmp = tempfile.mkdtemp() - view.presenter.create_project("test", tmp) - mock_get_dir.return_value = tmp + view = MainWindowView() + view.check_save_blacklist = MagicMock(return_value=False) + mock_overwrite = MagicMock(return_value=True) - with patch.object(view, "show_confirm_dialog", new=mock_overwrite): - assert view.get_project_folder() == tmp + tmp = tempfile.mkdtemp() + view.presenter.create_project("test", tmp) + mock_get_dir.return_value = tmp - # check overwrite is triggered if project already in folder - Path(tmp, "controls.json").touch() - with patch.object(view, "show_confirm_dialog", new=mock_overwrite): - assert view.get_project_folder() == tmp - mock_overwrite.assert_called_once() + with patch.object(view, "show_confirm_dialog", new=mock_overwrite): + assert view.get_project_folder() == tmp - def change_dir(*args, **kwargs): - """Change directory so mocked save_as doesn't recurse forever.""" - mock_get_dir.return_value = "OTHERPATH" + # check overwrite is triggered if project already in folder + Path(tmp, "controls.json").touch() + with patch.object(view, "show_confirm_dialog", new=mock_overwrite): + assert view.get_project_folder() == tmp + mock_overwrite.assert_called_once() - # check not saved if overwrite is cancelled - # to avoid infinite recursion (which only happens because of the mock), - # set the mock to change the directory to some other path once called - mock_overwrite = MagicMock(return_value=False, side_effect=change_dir) + def change_dir(*args, **kwargs): + """Change directory so mocked save_as doesn't recurse forever.""" + mock_get_dir.return_value = "OTHERPATH" - with patch.object(view, "show_confirm_dialog", new=mock_overwrite): - assert view.get_project_folder() == "OTHERPATH" + # check not saved if overwrite is cancelled + # to avoid infinite recursion (which only happens because of the mock), + # set the mock to change the directory to some other path once called + mock_overwrite = MagicMock(return_value=False, side_effect=change_dir) - mock_overwrite.assert_called_once() + with patch.object(view, "show_confirm_dialog", new=mock_overwrite): + assert view.get_project_folder() == "OTHERPATH" + + mock_overwrite.assert_called_once() @pytest.mark.parametrize("submenu_name", ["&File", "&Edit", "&Windows", "&Tools", "&Help"]) From 08933ee74492f613a00645cf8df1ac760db4bfd7 Mon Sep 17 00:00:00 2001 From: Stephen Nneji Date: Mon, 26 Jan 2026 10:10:45 +0000 Subject: [PATCH 2/2] Change to write log to custom logger but still log root --- rascal2/config.py | 4 ++-- rascal2/core/commands.py | 5 +++-- rascal2/dialogs/custom_file_editor.py | 10 ++++------ rascal2/dialogs/startup_dialog.py | 5 ++--- rascal2/ui/presenter.py | 13 ++++++------- tests/ui/test_presenter.py | 14 +++++++------- 6 files changed, 24 insertions(+), 27 deletions(-) diff --git a/rascal2/config.py b/rascal2/config.py index 51591f04..a7f90958 100644 --- a/rascal2/config.py +++ b/rascal2/config.py @@ -24,6 +24,7 @@ IMAGES_PATH = STATIC_PATH / "images" MATLAB_ARCH_FILE = pathlib.Path(SITE_PATH) / "matlab/engine/_arch.txt" EXAMPLES_TEMP_PATH = pathlib.Path(get_global_settings().fileName()).parent / "examples" +LOGGER = logging.getLogger("rascal2") def handle_scaling(): @@ -237,6 +238,5 @@ def get_matlab_path(self): if error: self.engine_output[:] = [] self.engine_output.append(Exception(error)) - logger = logging.getLogger("rascal_log") - logger.error(f"{error}. Attempt to read MATLAB _arch file failed {MATLAB_ARCH_FILE}.") + LOGGER.error(f"{error}. Attempt to read MATLAB _arch file failed {MATLAB_ARCH_FILE}.") return str(install_dir) diff --git a/rascal2/core/commands.py b/rascal2/core/commands.py index 85e60eed..2f712e61 100644 --- a/rascal2/core/commands.py +++ b/rascal2/core/commands.py @@ -1,7 +1,6 @@ """File for Qt commands.""" import copy -import logging from collections.abc import Callable from enum import IntEnum, unique @@ -9,6 +8,8 @@ from PyQt6 import QtGui from ratapi import ClassList +from rascal2.config import LOGGER + @unique class CommandID(IntEnum): @@ -69,7 +70,7 @@ def redo(self): except Exception as ex: self.new_result = self.old_result message = f"Error occurred when generating result preview:\n\n{ex}" - logging.error(message, exc_info=ex) + LOGGER.error(message, exc_info=ex) self.presenter.view.terminal_widget.write(message) self.presenter.model.update_results(self.new_result) else: diff --git a/rascal2/dialogs/custom_file_editor.py b/rascal2/dialogs/custom_file_editor.py index f904eae7..7410b34e 100644 --- a/rascal2/dialogs/custom_file_editor.py +++ b/rascal2/dialogs/custom_file_editor.py @@ -1,12 +1,11 @@ """Dialogs for editing custom files.""" -import logging from pathlib import Path from PyQt6 import Qsci, QtGui, QtWidgets from ratapi.utils.enums import Languages -from rascal2.config import EXAMPLES_PATH, MatlabHelper +from rascal2.config import EXAMPLES_PATH, LOGGER, MatlabHelper def edit_file(filename: str, language: Languages, parent: QtWidgets.QWidget): @@ -24,8 +23,7 @@ def edit_file(filename: str, language: Languages, parent: QtWidgets.QWidget): """ file = Path(filename) if not file.is_file(): - logger = logging.getLogger("rascal_log") - logger.error("Attempted to edit a custom file which does not exist!") + LOGGER.error("Attempted to edit a custom file which does not exist!") return dialog = CustomFileEditorDialog(file, language, parent) @@ -37,7 +35,7 @@ def edit_file_matlab(filename: str): try: engine = MatlabHelper().get_local_engine() except Exception as ex: - logging.error("Attempted to edit a file in MATLAB engine", exc_info=ex) + LOGGER.error("Attempted to edit a file in MATLAB engine", exc_info=ex) return engine.edit(str(filename)) @@ -131,5 +129,5 @@ def save_file(self): self.accept() except OSError as ex: message = f"Failed to save custom file to {self.file}.\n" - logging.error(message, exc_info=ex) + LOGGER.error(message, exc_info=ex) QtWidgets.QMessageBox.critical(self, "Save File", message, QtWidgets.QMessageBox.StandardButton.Ok) diff --git a/rascal2/dialogs/startup_dialog.py b/rascal2/dialogs/startup_dialog.py index 41580676..ab4fd9e1 100644 --- a/rascal2/dialogs/startup_dialog.py +++ b/rascal2/dialogs/startup_dialog.py @@ -1,10 +1,9 @@ -import logging import os from pathlib import Path from PyQt6 import QtCore, QtWidgets -from rascal2.config import EXAMPLES_PATH +from rascal2.config import EXAMPLES_PATH, LOGGER from rascal2.core.worker import Worker from rascal2.settings import update_recent_projects @@ -183,7 +182,7 @@ def project_start_failed(self, exception, args): folder_name = args[0] error = str(exception).strip().replace("\n", "") message = f"The Project ({folder_name}) could not be opened because:\n\n{error}" - logging.error(message, exc_info=exception) + LOGGER.error(message, exc_info=exception) QtWidgets.QMessageBox.critical(self, self.windowTitle(), message) diff --git a/rascal2/ui/presenter.py b/rascal2/ui/presenter.py index 3ddd2402..def2cbe9 100644 --- a/rascal2/ui/presenter.py +++ b/rascal2/ui/presenter.py @@ -1,4 +1,3 @@ -import logging import re import warnings from typing import Any @@ -6,7 +5,7 @@ import ratapi as rat import ratapi.wrappers -from rascal2.config import MatlabHelper, get_matlab_engine +from rascal2.config import LOGGER, MatlabHelper, get_matlab_engine from rascal2.core import commands from rascal2.core.enums import UnsavedReply from rascal2.core.runner import LogData, RATRunner @@ -124,7 +123,7 @@ def save_project(self, save_as: bool = False): try: self.model.save_project(to_path) except OSError as err: - logging.error(f"Failed to save project to {to_path}.\n", exc_info=err) + LOGGER.error(f"Failed to save project to {to_path}.\n", exc_info=err) else: update_recent_projects(self.model.save_path) self.view.undo_stack.setClean() @@ -158,7 +157,7 @@ def export_fits(self): try: write_result_to_zipped_csvs(save_file, results) except OSError as err: - logging.error(f"Failed to save fits to {save_file}.\n", exc_info=err) + LOGGER.error(f"Failed to save fits to {save_file}.\n", exc_info=err) def interrupt_terminal(self): """Send an interrupt signal to the RAT runner.""" @@ -232,9 +231,9 @@ def handle_results(self): def handle_interrupt(self): """Handle a RAT run being interrupted.""" if self.runner.error is None: - logging.info("RAT run interrupted!") + LOGGER.info("RAT run interrupted!") else: - logging.error("RAT run failed with exception.\n", exc_info=self.runner.error) + LOGGER.error("RAT run failed with exception.\n", exc_info=self.runner.error) self.view.handle_results() self.model.controls.delete_IPC() @@ -252,7 +251,7 @@ def handle_event(self): case rat.events.PlotEventData(): self.view.plot_widget.plot_with_blit(event) case LogData(): - logging.log(event.level, event.msg) + LOGGER.log(event.level, event.msg) def edit_project(self, updated_project: dict, preview: bool = True) -> None: """Edit the Project with a dictionary of attributes. diff --git a/tests/ui/test_presenter.py b/tests/ui/test_presenter.py index 2eca31e9..114c7916 100644 --- a/tests/ui/test_presenter.py +++ b/tests/ui/test_presenter.py @@ -47,14 +47,14 @@ def __init__(self): @pytest.fixture def presenter(): - with patch("rascal2.ui.presenter.logging", autospec=True) as mock_log: + with patch("rascal2.ui.presenter.LOGGER", autospec=True) as mock_log: pr = MainWindowPresenter(MockWindowView()) pr.runner = MagicMock() pr.model.controls = Controls() pr.model.project = MagicMock() pr.model.results = MagicMock() pr.model.save_path = "some_path/" - pr.logging = mock_log + pr.logger = mock_log yield pr @@ -128,7 +128,7 @@ def test_stop_run(presenter): presenter.runner = MagicMock() presenter.runner.error = None presenter.handle_interrupt() - presenter.logging.info.assert_called_once_with("RAT run interrupted!") + presenter.logger.info.assert_called_once_with("RAT run interrupted!") def test_run_error(presenter): @@ -136,7 +136,7 @@ def test_run_error(presenter): presenter.runner = MagicMock() presenter.runner.error = ValueError("Test error!") presenter.handle_interrupt() - presenter.logging.error.assert_called_once_with("RAT run failed with exception.\n", exc_info=presenter.runner.error) + presenter.logger.error.assert_called_once_with("RAT run failed with exception.\n", exc_info=presenter.runner.error) @pytest.mark.parametrize( @@ -169,7 +169,7 @@ def test_handle_progress_event(presenter): def test_handle_log_data(presenter): presenter.runner.events = [LogData(10, "Test log!")] presenter.handle_event() - presenter.logging.log.assert_called_with(10, "Test log!") + presenter.logger.log.assert_called_with(10, "Test log!") @pytest.mark.parametrize("function", ["create_project", "load_project", "load_r1_project"]) @@ -250,11 +250,11 @@ def test_export_fits(mock_write_result, presenter): error = OSError("Test Error") mock_write_result.side_effect = error presenter.view.get_save_file = MagicMock(return_value=test_zip_file) - presenter.logging.error = MagicMock() + presenter.logger.error = MagicMock() presenter.export_fits() mock_write_result.assert_called_once_with(test_zip_file, presenter.model.results) - presenter.logging.error.assert_called_once_with("Failed to save fits to test.zip.\n", exc_info=error) + presenter.logger.error.assert_called_once_with("Failed to save fits to test.zip.\n", exc_info=error) # If we do not have any results, don't ask for a file presenter.view.get_save_file.reset_mock()