From bbd75e287662bd33d2e16f180c85d6b38d6a84e1 Mon Sep 17 00:00:00 2001 From: Maria Patrou Date: Tue, 15 Aug 2023 15:12:13 -0400 Subject: [PATCH 01/10] configuration mechanism added to add user settings --- src/shiver/configuration_template.ini | 4 + src/shiver/models/configuration.py | 82 +++++++++++++ src/shiver/views/mainwindow.py | 17 ++- tests/conftest.py | 16 +++ tests/models/test_configuration.py | 164 ++++++++++++++++++++++++++ 5 files changed, 281 insertions(+), 2 deletions(-) create mode 100644 src/shiver/configuration_template.ini create mode 100644 src/shiver/models/configuration.py create mode 100644 tests/models/test_configuration.py diff --git a/src/shiver/configuration_template.ini b/src/shiver/configuration_template.ini new file mode 100644 index 00000000..6fee0876 --- /dev/null +++ b/src/shiver/configuration_template.ini @@ -0,0 +1,4 @@ +[generate_tab.oncat] +oncat_url = https://oncat.ornl.gov +client_id = 99025bb3-ce06-4f4b-bcf2-36ebf925cd1d +use_notes = 1 diff --git a/src/shiver/models/configuration.py b/src/shiver/models/configuration.py new file mode 100644 index 00000000..b40fc561 --- /dev/null +++ b/src/shiver/models/configuration.py @@ -0,0 +1,82 @@ +"""Module to load the the settings from SHOME/.shiver/configuration.ini file + +Will fall back to a default""" +import os +import shutil + +from configparser import ConfigParser +from pathlib import Path +from mantid.kernel import Logger + +logger = Logger("SHIVER") + + +class ConfigurationModel: + """Configuration Data""" + + def __init__(self, user_path=None): + """initialization of configuration mechanism""" + # capture the current state + self.valid = False + + # locate the template configuration file + project_directory = Path(__file__).resolve().parent.parent + self.template_file_path = os.path.join(project_directory, "configuration_template.ini") + if not os.path.exists(self.template_file_path): + logger.error(f"Template configuration file: {self.template_file_path} is missing!") + + # retrieve the file path of the file + if user_path: + self.config_file_path = user_path + else: + # default path + self.config_file_path = os.path.join(Path.home(), ".shiver", "configuration.ini") + logger.information(f"{self.config_file_path} with be used") + + # file does not exist create it from template + if not os.path.exists(self.config_file_path): + shutil.copy2(self.template_file_path, self.config_file_path) + + self.config = ConfigParser() + # parse the file + try: + self.config.read(self.config_file_path) + # validate the file has the all the latest variables + self.validate() + self.valid = True + except ValueError as err: + logger.error(str(err)) + logger.error(f"Problem with the file: {self.config_file_path}") + + def get_data(self, section, name=None): + """retrieves the configuration data for a variable with name""" + try: + if name: + return self.config[section][name] + return self.config[section] + except KeyError as err: + # requested section/field do not exist + logger.error(str(err)) + return None + + def validate(self): + """validates that the fields exist at the config_file_path and writes any missing fields/data + using the template configuration file: configuration_template.ini as a guide""" + template_config = ConfigParser() + template_config.read(self.template_file_path) + for section in template_config.sections(): + # if section is missing + if section not in self.config.sections(): + # copy the whole section + self.config.add_section(section) + + for field in template_config[section]: + if field not in self.config[section]: + # copy the field + self.config[section][field] = template_config[section][field] + with open(self.config_file_path, "w", encoding="utf8") as config_file: + self.config.write(config_file) + + def is_valid(self): + """returns the configuration state""" + return self.valid diff --git a/src/shiver/views/mainwindow.py b/src/shiver/views/mainwindow.py index f1e4aa4f..1316eb1e 100644 --- a/src/shiver/views/mainwindow.py +++ b/src/shiver/views/mainwindow.py @@ -2,7 +2,7 @@ Main Qt window for shiver """ -from qtpy.QtWidgets import QHBoxLayout, QVBoxLayout, QWidget, QTabWidget, QPushButton +from qtpy.QtWidgets import QHBoxLayout, QVBoxLayout, QWidget, QTabWidget, QPushButton, QMessageBox from mantidqt.widgets.algorithmprogress import AlgorithmProgressWidget from shiver.presenters.histogram import HistogramPresenter from shiver.models.histogram import HistogramModel @@ -12,6 +12,7 @@ from shiver.views.generate import Generate from shiver.views.corrections import Corrections from shiver.models.help import help_function +from shiver.models.configuration import ConfigurationModel class MainWindow(QWidget): @@ -20,8 +21,20 @@ class MainWindow(QWidget): def __init__(self, parent=None): super().__init__(parent) - self.tabs = QTabWidget() + self.config = ConfigurationModel() + if not self.config.is_valid(): + conf_dialog = QMessageBox(self) + conf_dialog.setWindowTitle("Error with configuration settings!") + msg = ( + f"Update your file: {self.config.config_file_path}", + "with the latest settings found here:", + f"{self.config.template_file_path} and start the application again.", + ) + conf_dialog.setText(" ".join(msg)) + conf_dialog.exec() + return + self.tabs = QTabWidget() histogram = Histogram(self) histogram_model = HistogramModel() self.histogram_presenter = HistogramPresenter(histogram, histogram_model) diff --git a/tests/conftest.py b/tests/conftest.py index c22bff39..466adb3f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,7 @@ """pytest config""" +import os +from configparser import ConfigParser + import pytest from mantid.simpleapi import mtd from shiver import Shiver @@ -16,3 +19,16 @@ def shiver_app(): def clear_ads(): """clear the ADS after every test""" mtd.clear() + + +@pytest.fixture(scope="session") +def user_conf_file(tmp_path_factory, request): + """Fixture to create a custom configuration file in tmp_path""" + # custom configuration file + config_data = request.param + user_config = ConfigParser(allow_no_value=True) + user_config.read_string(config_data) + user_path = os.path.join(tmp_path_factory.mktemp("data"), "test_config.ini") + with open(user_path, "w", encoding="utf8") as config_file: + user_config.write(config_file) + return user_path diff --git a/tests/models/test_configuration.py b/tests/models/test_configuration.py new file mode 100644 index 00000000..be7bae68 --- /dev/null +++ b/tests/models/test_configuration.py @@ -0,0 +1,164 @@ +"""Tests for ConfigurationModel""" +import os +from configparser import ConfigParser +from pathlib import Path + +import pytest +from shiver.models.configuration import ConfigurationModel + + +def test_config_path_default(): + """Test configuration default file path""" + config = ConfigurationModel() + assert config.config_file_path.endswith(".shiver/configuration.ini") is True + # check the valid state + assert config.is_valid() + assert config.valid == config.is_valid() + + +def test_config_path_does_not_exist(tmp_path): + """Test configuration user-defined file path that does not exist""" + user_path = os.path.join(tmp_path, "test_config.ini") + assert not os.path.exists(user_path) + + config = ConfigurationModel(user_path) + # check if the file is created and not exists + assert os.path.exists(user_path) + assert config.is_valid() + + +def test_config_path_invalid_format(): + """Test configuration user-defined file path that does not exist""" + user_path = os.path.join( + os.path.dirname(os.path.abspath(__file__)), "../data/mde/merged_mde_MnO_25meV_5K_unpol_178921-178926.nxs" + ) + config = ConfigurationModel(user_path) + # check if the file exists + assert not config.is_valid() + + +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [generate_tab.oncat] + oncat_url = https://oncat.ornl.gov + client_id = 99025bb3-ce06-4f4b-bcf2-36ebf925cd1d + use_notes = 1 + + """ + ], + indirect=True, +) +def test_field_validate_fields_exist(user_conf_file): + """Test configuration validate all fields exist with the same values as templates + Note: update the parameters if the fields increase""" + # read the custom configuration file + user_config = ConfigurationModel(user_conf_file) + + assert user_config.config_file_path.endswith(user_conf_file) is True + # check if the file exists + assert os.path.exists(user_conf_file) + + # check all fields are the same as the configuration template file + project_directory = Path(__file__).resolve().parent.parent.parent + template_file_path = os.path.join(project_directory, "src", "shiver", "configuration_template.ini") + template_config = ConfigParser() + template_config.read(template_file_path) + for section in user_config.config.sections(): + for field in user_config.config[section]: + assert user_config.config[section][field] == template_config[section][field] + + +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [generate_tab.oncat] + oncat_url = test_url + client_id = 0000-0000 + use_notes = 1 + """ + ], + indirect=True, +) +def test_field_validate_fields_same(user_conf_file): + """Test configuration validate all fields exist with their values; different from the template""" + + # read the custom configuration file + user_config = ConfigurationModel(user_conf_file) + + # check if the file exists + assert os.path.exists(user_conf_file) + + # check all field values have the same values as the user configuration file + assert user_config.get_data("generate_tab.oncat", "oncat_url") == "test_url" + assert user_config.get_data("generate_tab.oncat", "client_id") == "0000-0000" + assert user_config.get_data("generate_tab.oncat", "use_notes") == "1" + + +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [generate_tab.oncat] + client_id = 0000-0000 + """ + ], + indirect=True, +) +def test_field_validate_fields_missing(user_conf_file): + """Test configuration validate missing fields added from the template""" + # read the custom configuration file + + user_config = ConfigurationModel(user_conf_file) + + # check if the file exists + assert os.path.exists(user_conf_file) + + # check all field values have the same values as the user configuration file + assert user_config.get_data("generate_tab.oncat", "oncat_url") == "https://oncat.ornl.gov" + assert user_config.get_data("generate_tab.oncat", "client_id") == "0000-0000" + assert user_config.get_data("generate_tab.oncat", "use_notes") == "1" + + +@pytest.mark.parametrize("user_conf_file", ["""[generate_tab.oncat]"""], indirect=True) +def test_get_data_valid(user_conf_file): + """Test configuration get_data - valid""" + config = ConfigurationModel(user_conf_file) + assert config.config_file_path.endswith(user_conf_file) is True + # get the data + # section + assert len(config.get_data("generate_tab.oncat", "")) == 3 + # fields + assert config.get_data("generate_tab.oncat", "oncat_url") == "https://oncat.ornl.gov" + assert config.get_data("generate_tab.oncat", "client_id") == "99025bb3-ce06-4f4b-bcf2-36ebf925cd1d" + assert config.get_data("generate_tab.oncat", "use_notes") == "1" + + assert config.is_valid() + + +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [generate_tab.oncat] + oncat_url = test_url + client_id = 0000-0000 + use_notes = 1 + """ + ], + indirect=True, +) +def test_get_data_invalid(user_conf_file): + """Test configuration get_data - invalid""" + # read the custom configuration file + config = ConfigurationModel(user_conf_file) + assert config.config_file_path.endswith(user_conf_file) is True + + # section + assert config.get_data("section_not_here", "") is None + + assert len(config.get_data("generate_tab.oncat", "")) == 3 + # field + assert config.get_data("generate_tab.oncat", "field_not_here") is None From 11fe1f56833f2d15c6d72ebc3947dfd7bfce7174 Mon Sep 17 00:00:00 2001 From: Maria Patrou Date: Tue, 15 Aug 2023 15:31:34 -0400 Subject: [PATCH 02/10] add case where directory structure does not exist, test included --- src/shiver/models/configuration.py | 4 +++- tests/models/test_configuration.py | 13 ++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/shiver/models/configuration.py b/src/shiver/models/configuration.py index b40fc561..144050cc 100644 --- a/src/shiver/models/configuration.py +++ b/src/shiver/models/configuration.py @@ -24,7 +24,6 @@ def __init__(self, user_path=None): self.template_file_path = os.path.join(project_directory, "configuration_template.ini") if not os.path.exists(self.template_file_path): logger.error(f"Template configuration file: {self.template_file_path} is missing!") - # retrieve the file path of the file if user_path: self.config_file_path = user_path @@ -35,6 +34,9 @@ def __init__(self, user_path=None): # file does not exist create it from template if not os.path.exists(self.config_file_path): + # if directory structure does not exist create it + if not os.path.exists(os.path.dirname(self.config_file_path)): + os.makedirs(os.path.dirname(self.config_file_path)) shutil.copy2(self.template_file_path, self.config_file_path) self.config = ConfigParser() diff --git a/tests/models/test_configuration.py b/tests/models/test_configuration.py index be7bae68..a81b9117 100644 --- a/tests/models/test_configuration.py +++ b/tests/models/test_configuration.py @@ -16,13 +16,24 @@ def test_config_path_default(): assert config.valid == config.is_valid() +def test_config_path_in_folder(tmp_path): + """Test configuration configuration user-defined file path that does not exist in a new directory""" + user_path = os.path.join(tmp_path, "temp2", "test_config.ini") + assert not os.path.exists(user_path) + + config = ConfigurationModel(user_path) + # check if the file exists now + assert os.path.exists(user_path) + assert config.is_valid() + + def test_config_path_does_not_exist(tmp_path): """Test configuration user-defined file path that does not exist""" user_path = os.path.join(tmp_path, "test_config.ini") assert not os.path.exists(user_path) config = ConfigurationModel(user_path) - # check if the file is created and not exists + # check if the file is exists now assert os.path.exists(user_path) assert config.is_valid() From 8070b8926e335ba27873bbc33f38de82d627136b Mon Sep 17 00:00:00 2001 From: Maria Patrou Date: Tue, 15 Aug 2023 15:50:12 -0400 Subject: [PATCH 03/10] check whether the template conf file is missing and return error --- src/shiver/models/configuration.py | 39 ++++++++++++++++-------------- src/shiver/views/mainwindow.py | 2 +- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/shiver/models/configuration.py b/src/shiver/models/configuration.py index 144050cc..98567861 100644 --- a/src/shiver/models/configuration.py +++ b/src/shiver/models/configuration.py @@ -22,8 +22,7 @@ def __init__(self, user_path=None): # locate the template configuration file project_directory = Path(__file__).resolve().parent.parent self.template_file_path = os.path.join(project_directory, "configuration_template.ini") - if not os.path.exists(self.template_file_path): - logger.error(f"Template configuration file: {self.template_file_path} is missing!") + # retrieve the file path of the file if user_path: self.config_file_path = user_path @@ -32,23 +31,26 @@ def __init__(self, user_path=None): self.config_file_path = os.path.join(Path.home(), ".shiver", "configuration.ini") logger.information(f"{self.config_file_path} with be used") - # file does not exist create it from template - if not os.path.exists(self.config_file_path): - # if directory structure does not exist create it - if not os.path.exists(os.path.dirname(self.config_file_path)): - os.makedirs(os.path.dirname(self.config_file_path)) - shutil.copy2(self.template_file_path, self.config_file_path) + # if template conf file path exists + if os.path.exists(self.template_file_path): + # file does not exist create it from template + if not os.path.exists(self.config_file_path): + # if directory structure does not exist create it + if not os.path.exists(os.path.dirname(self.config_file_path)): + os.makedirs(os.path.dirname(self.config_file_path)) + shutil.copy2(self.template_file_path, self.config_file_path) - self.config = ConfigParser() - # parse the file - try: - self.config.read(self.config_file_path) - # validate the file has the all the latest variables - self.validate() - self.valid = True - except ValueError as err: - logger.error(str(err)) - logger.error(f"Problem with the file: {self.config_file_path}") + self.config = ConfigParser() + # parse the file + try: + self.config.read(self.config_file_path) + # validate the file has the all the latest variables + self.validate() + except ValueError as err: + logger.error(str(err)) + logger.error(f"Problem with the file: {self.config_file_path}") + else: + logger.error(f"Template configuration file: {self.template_file_path} is missing!") def get_data(self, section, name=None): """retrieves the configuration data for a variable with name""" @@ -78,6 +80,7 @@ def validate(self): self.config[section][field] = template_config[section][field] with open(self.config_file_path, "w", encoding="utf8") as config_file: self.config.write(config_file) + self.valid = True def is_valid(self): """returns the configuration state""" diff --git a/src/shiver/views/mainwindow.py b/src/shiver/views/mainwindow.py index 1316eb1e..1557c0e3 100644 --- a/src/shiver/views/mainwindow.py +++ b/src/shiver/views/mainwindow.py @@ -26,7 +26,7 @@ def __init__(self, parent=None): conf_dialog = QMessageBox(self) conf_dialog.setWindowTitle("Error with configuration settings!") msg = ( - f"Update your file: {self.config.config_file_path}", + f"Check and update your file: {self.config.config_file_path}", "with the latest settings found here:", f"{self.config.template_file_path} and start the application again.", ) From 955aa554dc9e1d8b47dc86040f7470fd2d47c409 Mon Sep 17 00:00:00 2001 From: Maria Patrou Date: Wed, 16 Aug 2023 14:21:08 -0400 Subject: [PATCH 04/10] oncat configuration settings added, related tests updated, imports --- src/shiver/configuration_template.ini | 14 ++++++++- src/shiver/presenters/configuration.py | 22 ++++++++++++++ src/shiver/views/generate.py | 6 ++++ src/shiver/views/mainwindow.py | 16 +++++++++- src/shiver/views/oncat.py | 30 ++++++++++++------ tests/models/test_configuration.py | 6 ++-- tests/views/test_oncat.py | 42 +++++++++++++++++++++----- 7 files changed, 114 insertions(+), 22 deletions(-) create mode 100644 src/shiver/presenters/configuration.py diff --git a/src/shiver/configuration_template.ini b/src/shiver/configuration_template.ini index 6fee0876..ef9dae71 100644 --- a/src/shiver/configuration_template.ini +++ b/src/shiver/configuration_template.ini @@ -1,4 +1,16 @@ [generate_tab.oncat] +#url to oncat portal oncat_url = https://oncat.ornl.gov +#client id for on cat +# NOTE: the client ID is unique for Shiver client_id = 99025bb3-ce06-4f4b-bcf2-36ebf925cd1d -use_notes = 1 +#boolaen value: True or False for oncat files +#the flag indicates that the names of the datasets are stored in the Notes/Comments +use_notes = False + +[main_tab.plot] +full_title = True +logarithmic_intensity = False + +[global.other] +help_url = https://neutrons.github.io/Shiver/GUI/ diff --git a/src/shiver/presenters/configuration.py b/src/shiver/presenters/configuration.py new file mode 100644 index 00000000..c7274a58 --- /dev/null +++ b/src/shiver/presenters/configuration.py @@ -0,0 +1,22 @@ +"""Presenter for Configuration""" + + +class ConfigurationPresenter: + """Configuration Parameter presenter""" + + def __init__(self, model, view): + self._model = model + self._view = view + # connect get configuration settings from ConfigurationModel + self.view.connect_get_config_callback(self.model.get_data) + self.view.connect_valid_config_callback(self.model.is_valid) + + @property + def view(self): + """Return the view for this presenter""" + return self._view + + @property + def model(self): + """Return the model for this presenter""" + return self._model diff --git a/src/shiver/views/generate.py b/src/shiver/views/generate.py index edc25aa5..ea46d6d2 100644 --- a/src/shiver/views/generate.py +++ b/src/shiver/views/generate.py @@ -104,6 +104,12 @@ def __init__(self, parent=None): self.mde_type_widget.check_mde_name() self.raw_data_widget.check_file_input() + def get_config_callback(self, section, field): + """get configuration settings for a section or field""" + if self.parent(): + return self.parent().get_config_callback(section, field) + return None + def generate_mde_finish_callback(self, activate): """Toggle the generate button disabled state.""" if not activate: diff --git a/src/shiver/views/mainwindow.py b/src/shiver/views/mainwindow.py index 1557c0e3..fcaedbac 100644 --- a/src/shiver/views/mainwindow.py +++ b/src/shiver/views/mainwindow.py @@ -13,6 +13,7 @@ from shiver.views.corrections import Corrections from shiver.models.help import help_function from shiver.models.configuration import ConfigurationModel +from shiver.presenters.configuration import ConfigurationPresenter class MainWindow(QWidget): @@ -21,8 +22,13 @@ class MainWindow(QWidget): def __init__(self, parent=None): super().__init__(parent) + # config callbacks + self.get_config_callback = None + self.valid_config_callback = None + self.config = ConfigurationModel() - if not self.config.is_valid(): + self.conf_presenter = ConfigurationPresenter(self.config, self) + if not self.valid_config_callback(): conf_dialog = QMessageBox(self) conf_dialog.setWindowTitle("Error with configuration settings!") msg = ( @@ -82,3 +88,11 @@ def handle_help(self): else: context = "" help_function(context=context) + + def connect_get_config_callback(self, callback): + """Connect the callback for generating the MDE""" + self.get_config_callback = callback + + def connect_valid_config_callback(self, callback): + """Connect the callback for generating the MDE""" + self.valid_config_callback = callback diff --git a/src/shiver/views/oncat.py b/src/shiver/views/oncat.py index 6f0ca1d0..1dbc32ad 100644 --- a/src/shiver/views/oncat.py +++ b/src/shiver/views/oncat.py @@ -16,11 +16,6 @@ ) from qtpy.QtCore import QTimer -# CONSTANTS -# NOTE: the client ID is unique for Shiver -ONCAT_URL = "https://oncat.ornl.gov" -CLIENT_ID = "99025bb3-ce06-4f4b-bcf2-36ebf925cd1d" - class OncatToken: """Class to hold OnCat token""" @@ -53,15 +48,16 @@ def write_token(self, token): class OnCatAgent: """Agent to interface with OnCat""" - def __init__(self) -> None: + def __init__(self, oncat_url, client_id, use_notes=False) -> None: """Initialize OnCat agent""" + self._use_notes = use_notes user_home_dir = os.path.expanduser("~") self._token = OncatToken( os.path.abspath(f"{user_home_dir}/.shiver/oncat_token.json"), ) self._agent = pyoncat.ONCat( - ONCAT_URL, - client_id=CLIENT_ID, + oncat_url, + client_id=client_id, # Pass in token getter/setter callbacks here: token_getter=self._token.read_token, token_setter=self._token.write_token, @@ -149,6 +145,7 @@ def get_datasets(self, facility: str, instrument: str, ipts: int) -> list: self._agent, ipts_number=ipts, instrument=instrument, + use_notes=self._use_notes, facility=facility, ) @@ -183,6 +180,12 @@ def __init__(self, parent=None, error_message_callback=None): self.error_message_callback = error_message_callback + def get_config_settings(self, section, field): + """get configuration settings for a section or field""" + if self.parent(): + return self.parent().get_config_settings(section, field) + return None + def accept(self): """Accept""" # login to OnCat @@ -276,7 +279,10 @@ def __init__(self, parent=None): self.error_message_callback = None # OnCat agent - self.oncat_agent = OnCatAgent() + oncat_url = self.get_config_settings("generate_tab.oncat", "oncat_url") + client_id = self.get_config_settings("generate_tab.oncat", "client_id") + use_notes = self.get_config_settings("generate_tab.oncat", "use_notes") == "True" + self.oncat_agent = OnCatAgent(oncat_url, client_id, use_notes) # Sync with remote self.sync_with_remote(refresh=True) @@ -448,6 +454,12 @@ def update_datasets(self): self.dataset.clear() self.dataset.addItems(sorted(dataset_list)) + def get_config_settings(self, section, field): + """get configuration settings for a section or field""" + if self.parent(): + return self.parent().get_config_callback(section, field) + return None + # The following are utility functions migrated from the original # oncat_util.py diff --git a/tests/models/test_configuration.py b/tests/models/test_configuration.py index a81b9117..1c921a73 100644 --- a/tests/models/test_configuration.py +++ b/tests/models/test_configuration.py @@ -55,7 +55,7 @@ def test_config_path_invalid_format(): [generate_tab.oncat] oncat_url = https://oncat.ornl.gov client_id = 99025bb3-ce06-4f4b-bcf2-36ebf925cd1d - use_notes = 1 + use_notes = False """ ], @@ -130,7 +130,7 @@ def test_field_validate_fields_missing(user_conf_file): # check all field values have the same values as the user configuration file assert user_config.get_data("generate_tab.oncat", "oncat_url") == "https://oncat.ornl.gov" assert user_config.get_data("generate_tab.oncat", "client_id") == "0000-0000" - assert user_config.get_data("generate_tab.oncat", "use_notes") == "1" + assert user_config.get_data("generate_tab.oncat", "use_notes") == "False" @pytest.mark.parametrize("user_conf_file", ["""[generate_tab.oncat]"""], indirect=True) @@ -144,7 +144,7 @@ def test_get_data_valid(user_conf_file): # fields assert config.get_data("generate_tab.oncat", "oncat_url") == "https://oncat.ornl.gov" assert config.get_data("generate_tab.oncat", "client_id") == "99025bb3-ce06-4f4b-bcf2-36ebf925cd1d" - assert config.get_data("generate_tab.oncat", "use_notes") == "1" + assert config.get_data("generate_tab.oncat", "use_notes") == "False" assert config.is_valid() diff --git a/tests/views/test_oncat.py b/tests/views/test_oncat.py index 14350cfc..e72a33ff 100644 --- a/tests/views/test_oncat.py +++ b/tests/views/test_oncat.py @@ -1,6 +1,10 @@ #!/usr/bin/env python # pylint: disable=all """Test the views for the ONCat application.""" +import os +from pathlib import Path +from configparser import ConfigParser + import pytest import pyoncat from qtpy.QtWidgets import QGroupBox @@ -12,11 +16,18 @@ get_data_from_oncat, get_dataset_names, get_dataset_info, - ONCAT_URL, - CLIENT_ID, ) +def get_configuration_settings(): + """get configuration settings from the configuration_template file""" + template_config = ConfigParser() + project_directory = Path(__file__).resolve().parent.parent.parent + template_file_path = os.path.join(project_directory, "src", "shiver", "configuration_template.ini") + template_config.read(template_file_path) + return template_config + + class MockRecord: def __init__(self, *args, **kwargs) -> None: pass @@ -62,8 +73,12 @@ def mock_get_dataset_names(*args, **kwargs): # mockey patch class pyoncat.ONCat monkeypatch.setattr("pyoncat.ONCat", MockONcat) + # get configuration setting from the configuration_template file + template_config = get_configuration_settings() # test the class - agent = OnCatAgent() + agent = OnCatAgent( + template_config["generate_tab.oncat"]["oncat_url"], template_config["generate_tab.oncat"]["client_id"] + ) # test login agent.login("test_login", "test_password") # test is_connected @@ -88,10 +103,10 @@ def login(self, *args, **kwargs) -> None: pass class DummyOnCatAgent: - def __init__(self) -> None: + def __init__(self, oncat_url, client_id) -> None: self._agent = pyoncat.ONCat( - ONCAT_URL, - client_id=CLIENT_ID, + oncat_url, + client_id=client_id, flow=pyoncat.RESOURCE_OWNER_CREDENTIALS_FLOW, ) @@ -110,7 +125,11 @@ def sync_with_remote(self, *args, **kwargs): class DummyParent(QGroupBox): def __init__(self, parent=None) -> None: super().__init__(parent) - self.oncat_agent = DummyOnCatAgent() + # get configuration setting from the configuration_template file + template_config = get_configuration_settings() + self.oncat_agent = DummyOnCatAgent( + template_config["generate_tab.oncat"]["oncat_url"], template_config["generate_tab.oncat"]["client_id"] + ) def sync_with_remote(self, *args, **kwargs): pass @@ -160,7 +179,7 @@ def test_oncat(monkeypatch, qtbot): # mockpatch OnCatAgent class MockOnCatAgent: - def __init__(self) -> None: + def __init__(self, oncat_url, client_id, use_notes) -> None: pass def login(self, *args, **kwargs) -> None: @@ -191,6 +210,13 @@ def mock_get_dataset_info(*args, **kwargs): monkeypatch.setattr("shiver.views.oncat.get_dataset_info", mock_get_dataset_info) + # mock get_oncat_url and client_id info + def mock_get_config_settings(*args, **kwargs): + # get configuration setting from the configuration_template file + return "" + + monkeypatch.setattr("shiver.views.oncat.Oncat.get_config_settings", mock_get_config_settings) + err_msgs = [] def error_message_callback(msg): From 8e32772e5e1320f4cf1b85397774c0b435b7ddf2 Mon Sep 17 00:00:00 2001 From: Maria Patrou Date: Thu, 17 Aug 2023 12:08:10 -0400 Subject: [PATCH 05/10] configuration mechamism simplification for integrating into code, oncat integration simplified, test updated --- src/shiver/__init__.py | 12 ++++ src/shiver/{models => }/configuration.py | 51 ++++++++------ src/shiver/models/help.py | 4 +- src/shiver/presenters/configuration.py | 22 ------ src/shiver/views/generate.py | 6 -- src/shiver/views/mainwindow.py | 30 +------- src/shiver/views/oncat.py | 30 +++----- tests/models/test_configuration.py | 88 +++++++++++++----------- tests/views/test_oncat.py | 55 ++++++++++----- 9 files changed, 140 insertions(+), 158 deletions(-) rename src/shiver/{models => }/configuration.py (75%) delete mode 100644 src/shiver/presenters/configuration.py diff --git a/src/shiver/__init__.py b/src/shiver/__init__.py index 0af88963..743c996e 100644 --- a/src/shiver/__init__.py +++ b/src/shiver/__init__.py @@ -15,6 +15,7 @@ import shiver.models.makeslice # noqa: F401 pylint: disable=unused-import import shiver.models.convert_dgs_to_single_mde # noqa: F401 pylint: disable=unused-import import shiver.models.generate_dgs_mde # noqa: F401 pylint: disable=unused-import +from shiver.configuration import Configuration from .version import __version__ # make sure matplotlib is correctly set before we import shiver @@ -31,6 +32,17 @@ def main(): """ Main entry point for Qt application """ + config = Configuration() + if not config.is_valid(): + msg = ( + "Error with configuration settings!", + f"Check and update your file: {config.config_file_path}", + "with the latest settings found here:", + f"{config.template_file_path} and start the application again.", + ) + + print(" ".join(msg)) + return app = QApplication(sys.argv) window = Shiver() window.show() diff --git a/src/shiver/models/configuration.py b/src/shiver/configuration.py similarity index 75% rename from src/shiver/models/configuration.py rename to src/shiver/configuration.py index 98567861..6395b04e 100644 --- a/src/shiver/models/configuration.py +++ b/src/shiver/configuration.py @@ -10,25 +10,24 @@ logger = Logger("SHIVER") +# configuration settings file path +CONFIG_PATH_FILE = os.path.join(Path.home(), ".shiver", "configuration.ini") -class ConfigurationModel: - """Configuration Data""" - def __init__(self, user_path=None): +class Configuration: + """Load and validate Configuration Data""" + + def __init__(self): """initialization of configuration mechanism""" # capture the current state self.valid = False # locate the template configuration file - project_directory = Path(__file__).resolve().parent.parent + project_directory = Path(__file__).resolve().parent self.template_file_path = os.path.join(project_directory, "configuration_template.ini") # retrieve the file path of the file - if user_path: - self.config_file_path = user_path - else: - # default path - self.config_file_path = os.path.join(Path.home(), ".shiver", "configuration.ini") + self.config_file_path = CONFIG_PATH_FILE logger.information(f"{self.config_file_path} with be used") # if template conf file path exists @@ -52,17 +51,6 @@ def __init__(self, user_path=None): else: logger.error(f"Template configuration file: {self.template_file_path} is missing!") - def get_data(self, section, name=None): - """retrieves the configuration data for a variable with name""" - try: - if name: - return self.config[section][name] - return self.config[section] - except KeyError as err: - # requested section/field do not exist - logger.error(str(err)) - return None - def validate(self): """validates that the fields exist at the config_file_path and writes any missing fields/data using the template configuration file: configuration_template.ini as a guide""" @@ -85,3 +73,26 @@ def validate(self): def is_valid(self): """returns the configuration state""" return self.valid + + +def get_data(section, name=None): + """retrieves the configuration data for a variable with name""" + # default file path location + config_file_path = CONFIG_PATH_FILE + if os.path.exists(config_file_path): + config = ConfigParser() + # parse the file + config.read(config_file_path) + try: + if name: + value = config[section][name] + # in case of boolean string value cast it to bool + if value in ("True", "False"): + return value == "True" + return value + return config[section] + except KeyError as err: + # requested section/field do not exist + logger.error(str(err)) + return None + return None diff --git a/src/shiver/models/help.py b/src/shiver/models/help.py index c03bf4dc..44ad46d7 100644 --- a/src/shiver/models/help.py +++ b/src/shiver/models/help.py @@ -1,10 +1,12 @@ """ single help module """ import webbrowser +from shiver.configuration import get_data def help_function(context): """ open a browser with the appropriate help page """ + help_url = get_data("global.other", "help_url") if context: - webbrowser.open("https://neutrons.github.io/Shiver/GUI/") + webbrowser.open(help_url) diff --git a/src/shiver/presenters/configuration.py b/src/shiver/presenters/configuration.py deleted file mode 100644 index c7274a58..00000000 --- a/src/shiver/presenters/configuration.py +++ /dev/null @@ -1,22 +0,0 @@ -"""Presenter for Configuration""" - - -class ConfigurationPresenter: - """Configuration Parameter presenter""" - - def __init__(self, model, view): - self._model = model - self._view = view - # connect get configuration settings from ConfigurationModel - self.view.connect_get_config_callback(self.model.get_data) - self.view.connect_valid_config_callback(self.model.is_valid) - - @property - def view(self): - """Return the view for this presenter""" - return self._view - - @property - def model(self): - """Return the model for this presenter""" - return self._model diff --git a/src/shiver/views/generate.py b/src/shiver/views/generate.py index ea46d6d2..edc25aa5 100644 --- a/src/shiver/views/generate.py +++ b/src/shiver/views/generate.py @@ -104,12 +104,6 @@ def __init__(self, parent=None): self.mde_type_widget.check_mde_name() self.raw_data_widget.check_file_input() - def get_config_callback(self, section, field): - """get configuration settings for a section or field""" - if self.parent(): - return self.parent().get_config_callback(section, field) - return None - def generate_mde_finish_callback(self, activate): """Toggle the generate button disabled state.""" if not activate: diff --git a/src/shiver/views/mainwindow.py b/src/shiver/views/mainwindow.py index fcaedbac..90b552ff 100644 --- a/src/shiver/views/mainwindow.py +++ b/src/shiver/views/mainwindow.py @@ -2,7 +2,7 @@ Main Qt window for shiver """ -from qtpy.QtWidgets import QHBoxLayout, QVBoxLayout, QWidget, QTabWidget, QPushButton, QMessageBox +from qtpy.QtWidgets import QHBoxLayout, QVBoxLayout, QWidget, QTabWidget, QPushButton from mantidqt.widgets.algorithmprogress import AlgorithmProgressWidget from shiver.presenters.histogram import HistogramPresenter from shiver.models.histogram import HistogramModel @@ -12,8 +12,6 @@ from shiver.views.generate import Generate from shiver.views.corrections import Corrections from shiver.models.help import help_function -from shiver.models.configuration import ConfigurationModel -from shiver.presenters.configuration import ConfigurationPresenter class MainWindow(QWidget): @@ -22,24 +20,6 @@ class MainWindow(QWidget): def __init__(self, parent=None): super().__init__(parent) - # config callbacks - self.get_config_callback = None - self.valid_config_callback = None - - self.config = ConfigurationModel() - self.conf_presenter = ConfigurationPresenter(self.config, self) - if not self.valid_config_callback(): - conf_dialog = QMessageBox(self) - conf_dialog.setWindowTitle("Error with configuration settings!") - msg = ( - f"Check and update your file: {self.config.config_file_path}", - "with the latest settings found here:", - f"{self.config.template_file_path} and start the application again.", - ) - conf_dialog.setText(" ".join(msg)) - conf_dialog.exec() - return - self.tabs = QTabWidget() histogram = Histogram(self) histogram_model = HistogramModel() @@ -88,11 +68,3 @@ def handle_help(self): else: context = "" help_function(context=context) - - def connect_get_config_callback(self, callback): - """Connect the callback for generating the MDE""" - self.get_config_callback = callback - - def connect_valid_config_callback(self, callback): - """Connect the callback for generating the MDE""" - self.valid_config_callback = callback diff --git a/src/shiver/views/oncat.py b/src/shiver/views/oncat.py index 1dbc32ad..c0141c96 100644 --- a/src/shiver/views/oncat.py +++ b/src/shiver/views/oncat.py @@ -15,6 +15,7 @@ QDoubleSpinBox, ) from qtpy.QtCore import QTimer +from shiver.configuration import get_data class OncatToken: @@ -48,16 +49,20 @@ def write_token(self, token): class OnCatAgent: """Agent to interface with OnCat""" - def __init__(self, oncat_url, client_id, use_notes=False) -> None: + def __init__(self, use_notes=False) -> None: """Initialize OnCat agent""" + # get configuration settings self._use_notes = use_notes + self._oncat_url = get_data("generate_tab.oncat", "oncat_url") + self._client_id = get_data("generate_tab.oncat", "client_id") + user_home_dir = os.path.expanduser("~") self._token = OncatToken( os.path.abspath(f"{user_home_dir}/.shiver/oncat_token.json"), ) self._agent = pyoncat.ONCat( - oncat_url, - client_id=client_id, + self._oncat_url, + client_id=self._client_id, # Pass in token getter/setter callbacks here: token_getter=self._token.read_token, token_setter=self._token.write_token, @@ -180,12 +185,6 @@ def __init__(self, parent=None, error_message_callback=None): self.error_message_callback = error_message_callback - def get_config_settings(self, section, field): - """get configuration settings for a section or field""" - if self.parent(): - return self.parent().get_config_settings(section, field) - return None - def accept(self): """Accept""" # login to OnCat @@ -278,11 +277,9 @@ def __init__(self, parent=None): # error message callback self.error_message_callback = None + self.use_notes = get_data("generate_tab.oncat", "use_notes") # OnCat agent - oncat_url = self.get_config_settings("generate_tab.oncat", "oncat_url") - client_id = self.get_config_settings("generate_tab.oncat", "client_id") - use_notes = self.get_config_settings("generate_tab.oncat", "use_notes") == "True" - self.oncat_agent = OnCatAgent(oncat_url, client_id, use_notes) + self.oncat_agent = OnCatAgent(self.use_notes) # Sync with remote self.sync_with_remote(refresh=True) @@ -332,6 +329,7 @@ def get_suggested_selected_files(self) -> list: login=self.oncat_agent.get_agent_instance(), ipts_number=self.get_ipts_number(), instrument=self.get_instrument(), + use_notes=self.use_notes, facility=self.get_facility(), group_by_angle=group_by_angle, angle_bin=self.angle_target.value(), @@ -454,12 +452,6 @@ def update_datasets(self): self.dataset.clear() self.dataset.addItems(sorted(dataset_list)) - def get_config_settings(self, section, field): - """get configuration settings for a section or field""" - if self.parent(): - return self.parent().get_config_callback(section, field) - return None - # The following are utility functions migrated from the original # oncat_util.py diff --git a/tests/models/test_configuration.py b/tests/models/test_configuration.py index 1c921a73..0162f124 100644 --- a/tests/models/test_configuration.py +++ b/tests/models/test_configuration.py @@ -1,53 +1,48 @@ -"""Tests for ConfigurationModel""" +"""Tests for Configuration mechanism""" import os from configparser import ConfigParser from pathlib import Path import pytest -from shiver.models.configuration import ConfigurationModel +from shiver.configuration import Configuration, get_data def test_config_path_default(): """Test configuration default file path""" - config = ConfigurationModel() + config = Configuration() assert config.config_file_path.endswith(".shiver/configuration.ini") is True # check the valid state assert config.is_valid() assert config.valid == config.is_valid() -def test_config_path_in_folder(tmp_path): +def test_config_path_in_folder(monkeypatch, tmp_path): """Test configuration configuration user-defined file path that does not exist in a new directory""" + user_path = os.path.join(tmp_path, "temp2", "test_config.ini") assert not os.path.exists(user_path) - config = ConfigurationModel(user_path) + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_path) + + config = Configuration() # check if the file exists now assert os.path.exists(user_path) assert config.is_valid() -def test_config_path_does_not_exist(tmp_path): +def test_config_path_does_not_exist(monkeypatch, tmp_path): """Test configuration user-defined file path that does not exist""" user_path = os.path.join(tmp_path, "test_config.ini") assert not os.path.exists(user_path) - config = ConfigurationModel(user_path) + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_path) + + config = Configuration() # check if the file is exists now assert os.path.exists(user_path) assert config.is_valid() -def test_config_path_invalid_format(): - """Test configuration user-defined file path that does not exist""" - user_path = os.path.join( - os.path.dirname(os.path.abspath(__file__)), "../data/mde/merged_mde_MnO_25meV_5K_unpol_178921-178926.nxs" - ) - config = ConfigurationModel(user_path) - # check if the file exists - assert not config.is_valid() - - @pytest.mark.parametrize( "user_conf_file", [ @@ -61,11 +56,12 @@ def test_config_path_invalid_format(): ], indirect=True, ) -def test_field_validate_fields_exist(user_conf_file): +def test_field_validate_fields_exist(monkeypatch, user_conf_file): """Test configuration validate all fields exist with the same values as templates Note: update the parameters if the fields increase""" # read the custom configuration file - user_config = ConfigurationModel(user_conf_file) + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + user_config = Configuration() assert user_config.config_file_path.endswith(user_conf_file) is True # check if the file exists @@ -88,24 +84,27 @@ def test_field_validate_fields_exist(user_conf_file): [generate_tab.oncat] oncat_url = test_url client_id = 0000-0000 - use_notes = 1 + use_notes = True """ ], indirect=True, ) -def test_field_validate_fields_same(user_conf_file): +def test_field_validate_fields_same(monkeypatch, user_conf_file): """Test configuration validate all fields exist with their values; different from the template""" # read the custom configuration file - user_config = ConfigurationModel(user_conf_file) + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + user_config = Configuration() # check if the file exists assert os.path.exists(user_conf_file) + assert user_config.config_file_path == user_conf_file # check all field values have the same values as the user configuration file - assert user_config.get_data("generate_tab.oncat", "oncat_url") == "test_url" - assert user_config.get_data("generate_tab.oncat", "client_id") == "0000-0000" - assert user_config.get_data("generate_tab.oncat", "use_notes") == "1" + assert get_data("generate_tab.oncat", "oncat_url") == "test_url" + assert get_data("generate_tab.oncat", "client_id") == "0000-0000" + # cast to bool + assert get_data("generate_tab.oncat", "use_notes") is True @pytest.mark.parametrize( @@ -118,33 +117,37 @@ def test_field_validate_fields_same(user_conf_file): ], indirect=True, ) -def test_field_validate_fields_missing(user_conf_file): +def test_field_validate_fields_missing(monkeypatch, user_conf_file): """Test configuration validate missing fields added from the template""" - # read the custom configuration file - user_config = ConfigurationModel(user_conf_file) + # read the custom configuration file + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + user_config = Configuration() # check if the file exists assert os.path.exists(user_conf_file) + assert user_config.config_file_path == user_conf_file # check all field values have the same values as the user configuration file - assert user_config.get_data("generate_tab.oncat", "oncat_url") == "https://oncat.ornl.gov" - assert user_config.get_data("generate_tab.oncat", "client_id") == "0000-0000" - assert user_config.get_data("generate_tab.oncat", "use_notes") == "False" + assert get_data("generate_tab.oncat", "oncat_url") == "https://oncat.ornl.gov" + assert get_data("generate_tab.oncat", "client_id") == "0000-0000" + assert get_data("generate_tab.oncat", "use_notes") is False @pytest.mark.parametrize("user_conf_file", ["""[generate_tab.oncat]"""], indirect=True) -def test_get_data_valid(user_conf_file): +def test_get_data_valid(monkeypatch, user_conf_file): """Test configuration get_data - valid""" - config = ConfigurationModel(user_conf_file) + + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + config = Configuration() assert config.config_file_path.endswith(user_conf_file) is True # get the data # section - assert len(config.get_data("generate_tab.oncat", "")) == 3 + assert len(get_data("generate_tab.oncat", "")) == 3 # fields - assert config.get_data("generate_tab.oncat", "oncat_url") == "https://oncat.ornl.gov" - assert config.get_data("generate_tab.oncat", "client_id") == "99025bb3-ce06-4f4b-bcf2-36ebf925cd1d" - assert config.get_data("generate_tab.oncat", "use_notes") == "False" + assert get_data("generate_tab.oncat", "oncat_url") == "https://oncat.ornl.gov" + assert get_data("generate_tab.oncat", "client_id") == "99025bb3-ce06-4f4b-bcf2-36ebf925cd1d" + assert get_data("generate_tab.oncat", "use_notes") is False assert config.is_valid() @@ -161,15 +164,16 @@ def test_get_data_valid(user_conf_file): ], indirect=True, ) -def test_get_data_invalid(user_conf_file): +def test_get_data_invalid(monkeypatch, user_conf_file): """Test configuration get_data - invalid""" # read the custom configuration file - config = ConfigurationModel(user_conf_file) + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + config = Configuration() assert config.config_file_path.endswith(user_conf_file) is True # section - assert config.get_data("section_not_here", "") is None + assert get_data("section_not_here", "") is None - assert len(config.get_data("generate_tab.oncat", "")) == 3 + assert len(get_data("generate_tab.oncat", "")) == 3 # field - assert config.get_data("generate_tab.oncat", "field_not_here") is None + assert get_data("generate_tab.oncat", "field_not_here") is None diff --git a/tests/views/test_oncat.py b/tests/views/test_oncat.py index e72a33ff..8d818c09 100644 --- a/tests/views/test_oncat.py +++ b/tests/views/test_oncat.py @@ -73,12 +73,20 @@ def mock_get_dataset_names(*args, **kwargs): # mockey patch class pyoncat.ONCat monkeypatch.setattr("pyoncat.ONCat", MockONcat) - # get configuration setting from the configuration_template file - template_config = get_configuration_settings() + # mockey patch get_settings data + def mock_get_data(*args, **kwargs): + # get configuration setting from the configuration_template file + template_config = get_configuration_settings() + return template_config[args[1], args[2]] + + monkeypatch.setattr("shiver.configuration.get_data", mock_get_data) + # test the class - agent = OnCatAgent( - template_config["generate_tab.oncat"]["oncat_url"], template_config["generate_tab.oncat"]["client_id"] - ) + agent = OnCatAgent() + # test configuration settings are stored from template configuration file + assert agent._oncat_url == "https://oncat.ornl.gov" + assert agent._client_id == "99025bb3-ce06-4f4b-bcf2-36ebf925cd1d" + assert agent._use_notes is False # test login agent.login("test_login", "test_password") # test is_connected @@ -103,7 +111,10 @@ def login(self, *args, **kwargs) -> None: pass class DummyOnCatAgent: - def __init__(self, oncat_url, client_id) -> None: + def __init__(self) -> None: + template_config = get_configuration_settings() + oncat_url = template_config["generate_tab.oncat"]["oncat_url"] + client_id = template_config["generate_tab.oncat"]["client_id"] self._agent = pyoncat.ONCat( oncat_url, client_id=client_id, @@ -125,11 +136,7 @@ def sync_with_remote(self, *args, **kwargs): class DummyParent(QGroupBox): def __init__(self, parent=None) -> None: super().__init__(parent) - # get configuration setting from the configuration_template file - template_config = get_configuration_settings() - self.oncat_agent = DummyOnCatAgent( - template_config["generate_tab.oncat"]["oncat_url"], template_config["generate_tab.oncat"]["client_id"] - ) + self.oncat_agent = DummyOnCatAgent() def sync_with_remote(self, *args, **kwargs): pass @@ -174,12 +181,24 @@ def error_message_callback(msg): assert err_msgs[-1] == "Invalid username or password. Please try again." -def test_oncat(monkeypatch, qtbot): +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [generate_tab.oncat] + oncat_url = test_url + client_id = 0000-0000 + use_notes = False + """ + ], + indirect=True, +) +def test_oncat(monkeypatch, user_conf_file, qtbot): """Test the Oncat class.""" # mockpatch OnCatAgent class MockOnCatAgent: - def __init__(self, oncat_url, client_id, use_notes) -> None: + def __init__(self, use_notes) -> None: pass def login(self, *args, **kwargs) -> None: @@ -210,12 +229,8 @@ def mock_get_dataset_info(*args, **kwargs): monkeypatch.setattr("shiver.views.oncat.get_dataset_info", mock_get_dataset_info) - # mock get_oncat_url and client_id info - def mock_get_config_settings(*args, **kwargs): - # get configuration setting from the configuration_template file - return "" - - monkeypatch.setattr("shiver.views.oncat.Oncat.get_config_settings", mock_get_config_settings) + # mock get_oncat_url, client_id and use_notes info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) err_msgs = [] @@ -227,6 +242,8 @@ def error_message_callback(msg): oncat.connect_error_callback(error_message_callback) qtbot.addWidget(oncat) oncat.show() + # test use_notes are saved from configuration settings + assert oncat.use_notes is False # test connect status check assert oncat.connected_to_oncat is True # test get_suggested_path From eee4f87a2eadffd597ccd1ee2c587e7a929a4c3e Mon Sep 17 00:00:00 2001 From: Maria Patrou Date: Fri, 18 Aug 2023 08:50:41 -0400 Subject: [PATCH 06/10] template settings updates --- src/shiver/configuration_template.ini | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/shiver/configuration_template.ini b/src/shiver/configuration_template.ini index ef9dae71..c307484e 100644 --- a/src/shiver/configuration_template.ini +++ b/src/shiver/configuration_template.ini @@ -4,12 +4,13 @@ oncat_url = https://oncat.ornl.gov #client id for on cat # NOTE: the client ID is unique for Shiver client_id = 99025bb3-ce06-4f4b-bcf2-36ebf925cd1d -#boolaen value: True or False for oncat files +#boolean value: True or False for oncat files #the flag indicates that the names of the datasets are stored in the Notes/Comments use_notes = False [main_tab.plot] -full_title = True +#options: full prints dimension data, name_only, workspace title, None: no title is printed in plots +title = True logarithmic_intensity = False [global.other] From 25fdfea473a39e4583696e5acad3de1b02bacaec Mon Sep 17 00:00:00 2001 From: Maria Patrou Date: Mon, 21 Aug 2023 09:53:51 -0400 Subject: [PATCH 07/10] plots tests added/updated for configuration settings --- src/shiver/views/histogram.py | 25 ++- src/shiver/views/plots.py | 49 +++-- tests/views/test_plots.py | 384 +++++++++++++++++++++++++++++++++- 3 files changed, 430 insertions(+), 28 deletions(-) diff --git a/src/shiver/views/histogram.py b/src/shiver/views/histogram.py index 5933de28..b5c22a89 100644 --- a/src/shiver/views/histogram.py +++ b/src/shiver/views/histogram.py @@ -6,6 +6,7 @@ ) from qtpy.QtCore import Signal +from shiver.configuration import get_data from .loading_buttons import LoadingButtons from .histogram_parameters import HistogramParameter from .workspace_tables import InputWorkspaces, HistogramWorkspaces @@ -78,13 +79,7 @@ def make_slice_finish(self, ws_name, ndims): self.makeslice_finish_signal.emit(ws_name, ndims) def _make_slice_finish(self, ws_name, ndims): - display_name = self.plot_display_name_callback(ws_name, ndims) - min_intensity = self.histogram_parameters.dimensions.intensity_min.text() - max_intensity = self.histogram_parameters.dimensions.intensity_max.text() - intensity_limits = { - "min": float(min_intensity) if min_intensity != "" else None, - "max": float(max_intensity) if max_intensity != "" else None, - } + display_name, intensity_limits = self.get_plot_data(ws_name, ndims) do_default_plot(ws_name, ndims, display_name, intensity_limits) self.histogram_workspaces.histogram_workspaces.set_selected(ws_name) @@ -203,3 +198,19 @@ def unset_all(self): self.input_workspaces.mde_workspaces.unset_all() self.input_workspaces.norm_workspaces.deselect_all() self.set_field_invalid_state(self.input_workspaces.mde_workspaces) + + def get_plot_data(self, ws_name, ndims): + """Get display name and intensities data for plotting.""" + plot_title_preference = get_data("main_tab.plot", "title") + display_name = None + if plot_title_preference == "full": + display_name = self.plot_display_name_callback(ws_name, ndims) + if plot_title_preference == "name_only": + display_name = ws_name.name() + min_intensity = self.histogram_parameters.dimensions.intensity_min.text() + max_intensity = self.histogram_parameters.dimensions.intensity_max.text() + intensity_limits = { + "min": float(min_intensity) if min_intensity != "" else None, + "max": float(max_intensity) if max_intensity != "" else None, + } + return (display_name, intensity_limits) diff --git a/src/shiver/views/plots.py b/src/shiver/views/plots.py index f713cdf5..3d37d9f5 100644 --- a/src/shiver/views/plots.py +++ b/src/shiver/views/plots.py @@ -2,29 +2,44 @@ import matplotlib.pyplot as plt from mantidqt.widgets.sliceviewer.presenters.presenter import SliceViewer from mantidqt.plotting.functions import manage_workspace_names, plot_md_ws_from_names +from shiver.configuration import get_data @manage_workspace_names -def do_1d_plot(workspaces, display_name, intensity_limits=None): +def do_1d_plot(workspaces, display_name, intensity_limits=None, log_scale=False): """Create an 1D plot for the provided workspace""" fig = plot_md_ws_from_names(workspaces, False, False) plot_title = display_name if display_name else workspaces[0].name() min_limit = intensity_limits["min"] if intensity_limits is not None and "min" in intensity_limits else None max_limit = intensity_limits["max"] if intensity_limits is not None and "max" in intensity_limits else None - fig.axes[0].set_ylim(min_limit, max_limit) + + # logarithic case + if log_scale: + fig.axes[0].set_yscale("log") + + # y limits + if not (min_limit is None and max_limit is None): + fig.axes[0].set_ylim(min_limit, max_limit) + fig.canvas.manager.set_window_title(display_name) fig.axes[0].set_title(plot_title) return fig @manage_workspace_names -def do_colorfill_plot(workspaces, display_name=None, intensity_limits=None): +def do_colorfill_plot(workspaces, display_name=None, intensity_limits=None, log_scale=False): """Create a colormesh plot for the provided workspace""" fig, axis = plt.subplots(subplot_kw={"projection": "mantid"}) plot_title = display_name or workspaces[0].name() + # y limits min_limit = intensity_limits["min"] if intensity_limits is not None and "min" in intensity_limits else None max_limit = intensity_limits["max"] if intensity_limits is not None and "max" in intensity_limits else None - colormesh = axis.pcolormesh(workspaces[0], vmin=min_limit, vmax=max_limit) + + # logarithic case + scale_norm = "linear" + if log_scale: + scale_norm = "log" + colormesh = axis.pcolormesh(workspaces[0], vmin=min_limit, vmax=max_limit, norm=scale_norm) axis.set_title(plot_title) fig.canvas.manager.set_window_title(plot_title) @@ -34,7 +49,7 @@ def do_colorfill_plot(workspaces, display_name=None, intensity_limits=None): @manage_workspace_names -def do_slice_viewer(workspaces, parent=None, intensity_limits=None): +def do_slice_viewer(workspaces, parent=None, intensity_limits=None, log_scale=False): """Open sliceviewer for the provided workspace""" presenter = SliceViewer(ws=workspaces[0], parent=parent) @@ -48,10 +63,19 @@ def do_slice_viewer(workspaces, parent=None, intensity_limits=None): if intensity_limits is not None and intensity_limits["max"] is not None else presenter.view.data_view.colorbar.cmax_value ) - presenter.view.data_view.colorbar.cmin.setText(f"{min_limit:.4}") - presenter.view.data_view.colorbar.clim_changed() - presenter.view.data_view.colorbar.cmax.setText(f"{max_limit:.4}") - presenter.view.data_view.colorbar.clim_changed() + + # y limits + if not (min_limit is None and max_limit is None): + presenter.view.data_view.colorbar.cmin.setText(f"{min_limit:.4}") + presenter.view.data_view.colorbar.clim_changed() + presenter.view.data_view.colorbar.cmax.setText(f"{max_limit:.4}") + presenter.view.data_view.colorbar.clim_changed() + + # logarithic case + if log_scale: + norm_scale = "Log" + presenter.view.data_view.colorbar.norm.setCurrentText(norm_scale) + presenter.view.data_view.colorbar.norm_changed() presenter.view.show() return presenter.view @@ -59,10 +83,11 @@ def do_slice_viewer(workspaces, parent=None, intensity_limits=None): def do_default_plot(workspace, ndims, display_name=None, intensity_limits=None): """Create the default plot for the workspace and number of dimensions""" + log_scale = get_data("main_tab.plot", "logarithmic_intensity") if ndims == 1: - return do_1d_plot([workspace], display_name, intensity_limits) + return do_1d_plot([workspace], display_name, intensity_limits, log_scale) if ndims == 2: - return do_colorfill_plot([workspace], display_name, intensity_limits) + return do_colorfill_plot([workspace], display_name, intensity_limits, log_scale) if ndims in (3, 4): - return do_slice_viewer([workspace], intensity_limits=intensity_limits) + return do_slice_viewer([workspace], intensity_limits=intensity_limits, log_scale=log_scale) return None diff --git a/tests/views/test_plots.py b/tests/views/test_plots.py index 52f54d24..3d57a7d8 100644 --- a/tests/views/test_plots.py +++ b/tests/views/test_plots.py @@ -1,16 +1,33 @@ #!/usr/bin/env python """UI tests for Plots""" +import pytest + # pylint: disable=no-name-in-module from mantid.simpleapi import ( mtd, CreateMDHistoWorkspace, ) from shiver.views.plots import do_default_plot +from shiver.views.histogram import Histogram -def test_plot1d(qtbot): +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = name_only + logarithmic_intensity = False + """ + ], + indirect=True, +) +def test_plot1d(qtbot, user_conf_file, monkeypatch): """Test for 1D plot with intensities and display title""" + # mock get_oncat_url, client_id and use_notes info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + # clear mantid workspace mtd.clear() @@ -30,12 +47,66 @@ def test_plot1d(qtbot): fig = do_default_plot(workspace, 1, title, {"min": intensity_min, "max": intensity_max}) assert fig.axes[0].get_title() == title assert fig.axes[0].get_ylim() == (intensity_min, intensity_max) - qtbot.wait(500) + assert fig.axes[0].get_yscale() == "linear" + + qtbot.wait(100) + + +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = name_only + logarithmic_intensity = True + """ + ], + indirect=True, +) +def test_plot1d_scale(qtbot, user_conf_file, monkeypatch): + """Test for 1D plot with log scale""" + + # mock get_oncat_url, client_id and use_notes info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + + # clear mantid workspace + mtd.clear() + + workspace = CreateMDHistoWorkspace( + Dimensionality=1, + Extents="-3,3", + SignalInput=range(0, 10), + ErrorInput=range(0, 10), + NumberOfBins="10", + Names="Dim1", + Units="MomentumTransfer", + ) + + intensity_min = None + intensity_max = None + title = "1D Plot" + fig = do_default_plot(workspace, 1, title, {"min": intensity_min, "max": intensity_max}) + assert fig.axes[0].get_yscale() == "log" + qtbot.wait(100) -def test_plot2d(qtbot): +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = name_only + logarithmic_intensity = False + """ + ], + indirect=True, +) +def test_plot2d(qtbot, user_conf_file, monkeypatch): """Test for 2D plot with intensities and display title""" + # mock get_oncat_url, client_id and use_notes info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + # clear mantid workspace mtd.clear() @@ -54,13 +125,70 @@ def test_plot2d(qtbot): title = "2D Plot" fig = do_default_plot(workspace, 2, title, {"min": intensity_min, "max": intensity_max}) assert fig.axes[0].get_title() == title - assert fig.axes[0].collections[0].get_clim() == (intensity_min, intensity_max) - qtbot.wait(500) + assert fig.axes[1].collections[1].get_clim() == (intensity_min, intensity_max) + assert fig.axes[1].get_yscale() == "linear" + + qtbot.wait(100) + + +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = name_only + logarithmic_intensity = True + """ + ], + indirect=True, +) +def test_plot2d_scale(qtbot, user_conf_file, monkeypatch): + """Test for 2D plot with log scale""" + + # mock get_oncat_url, client_id and use_notes info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + + # clear mantid workspace + mtd.clear() + + workspace = CreateMDHistoWorkspace( + Dimensionality=2, + Extents="-2,2,-5,5", + SignalInput=range(0, 100), + ErrorInput=range(0, 100), + NumberOfBins="10,10", + Names="Dim1,Dim2", + Units="Momentum,Energy", + ) + + intensity_min = 0.001 + intensity_max = 1 + title = "2D Plot" + fig = do_default_plot(workspace, 2, title, {"min": intensity_min, "max": intensity_max}) + assert fig.axes[0].get_title() == title + assert fig.axes[1].collections[1].get_clim() == (intensity_min, intensity_max) + assert fig.axes[1].get_yscale() == "log" + + qtbot.wait(100) -def test_plot3d(qtbot): +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = name_only + logarithmic_intensity = False + """ + ], + indirect=True, +) +def test_plot3d(qtbot, user_conf_file, monkeypatch): """Test for 3D plot with intensities""" + # mock get_oncat_url, client_id and use_notes info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + # clear mantid workspace mtd.clear() @@ -80,13 +208,64 @@ def test_plot3d(qtbot): view = do_default_plot(workspace, 3, title, {"min": intensity_min, "max": intensity_max}) assert view.data_view.colorbar.cmin_value == intensity_min assert view.data_view.colorbar.cmax_value == intensity_max + assert view.data_view.colorbar.norm.currentText() == "Linear" - qtbot.wait(500) + qtbot.wait(100) + + +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = name_only + logarithmic_intensity = True + """ + ], + indirect=True, +) +def test_plot3d_scale(qtbot, user_conf_file, monkeypatch): + """Test for 3D plot with log scale""" + + # mock get_oncat_url, client_id and use_notes info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + + # clear mantid workspace + mtd.clear() + + workspace = CreateMDHistoWorkspace( + Dimensionality=3, + Extents="-5,5,-10,10,-20,20", + SignalInput=range(0, 1000), + ErrorInput=range(0, 1000), + NumberOfBins="10,10,10", + Names="Dim1,Dim2,Dim3", + Units="Energy,Momentum,Other", + ) + + title = None + view = do_default_plot(workspace, 3, title) + assert view.data_view.colorbar.norm.currentText() == "Log" + qtbot.wait(100) -def test_plot4d(qtbot): +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = name_only + logarithmic_intensity = False + """ + ], + indirect=True, +) +def test_plot4d(qtbot, user_conf_file, monkeypatch): """Test for 4D plot with intensities""" + # mock get_oncat_url, client_id and use_notes info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + # clear mantid workspace mtd.clear() @@ -106,13 +285,70 @@ def test_plot4d(qtbot): view = do_default_plot(workspace, 4, title, {"min": intensity_min, "max": intensity_max}) assert view.data_view.colorbar.cmin_value == intensity_min assert view.data_view.colorbar.cmax_value == intensity_max + assert view.data_view.colorbar.norm.currentText() == "Linear" + + qtbot.wait(100) + + +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = name_only + logarithmic_intensity = True + """ + ], + indirect=True, +) +def test_plot4d_invalid_scale(qtbot, user_conf_file, monkeypatch): + """Test for 4D plot with intensities""" + + # mock get_oncat_url, client_id and use_notes info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + + # clear mantid workspace + mtd.clear() + + workspace = CreateMDHistoWorkspace( + Dimensionality=4, + Extents="-5,5,-10,10,-20,20,-30,30", + SignalInput=range(0, 10000), + ErrorInput=range(0, 10000), + NumberOfBins="10,10,10,10", + Names="Dim1,Dim2,Dim3,Dim4", + Units="M,E,O,EX", + ) + + intensity_min = -10.34 + intensity_max = 12.09 + title = None + view = do_default_plot(workspace, 4, title, {"min": intensity_min, "max": intensity_max}) + # mantid plot updates user values if invalid are passed + assert view.data_view.colorbar.cmin_value == 0.0001 + assert view.data_view.colorbar.cmax_value == intensity_max + assert view.data_view.colorbar.norm.currentText() == "Log" qtbot.wait(500) -def test_plot5d(qtbot): +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = name_only + logarithmic_intensity = False + """ + ], + indirect=True, +) +def test_plot5d(qtbot, user_conf_file, monkeypatch): """Test for 5D plot with intensities -invalid""" + # mock get_oncat_url, client_id and use_notes info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + # clear mantid workspace mtd.clear() @@ -132,3 +368,133 @@ def test_plot5d(qtbot): view = do_default_plot(workspace, 5, title, {"min": intensity_min, "max": intensity_max}) assert view is None qtbot.wait(100) + + +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = name_only + logarithmic_intensity = False + """ + ], + indirect=True, +) +def test_plot_data_name_only(qtbot, user_conf_file, monkeypatch): + """Test plot inputs""" + + # mock get_oncat_url, client_id and use_notes info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + histogram = Histogram() + # clear mantid workspace + mtd.clear() + + workspace = CreateMDHistoWorkspace( + Dimensionality=4, + Extents="-3,3,-10,10,-20,20,-30,30", + SignalInput=range(0, 10000), + ErrorInput=range(0, 10000), + NumberOfBins="10,10,10,10", + Names="Dim1,Dim2,Dim3,Dim4", + Units="EnergyT,MomentumT,Other,Extra", + ) + + data = histogram.get_plot_data(workspace, 4) + assert data[0] == "workspace" + assert data[1]["min"] is None + assert data[1]["max"] is None + + qtbot.wait(100) + + +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = full + logarithmic_intensity = False + """ + ], + indirect=True, +) +def test_plot_data_full(qtbot, user_conf_file, monkeypatch): + """Test plot inputs""" + + # mock plot info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + histogram = Histogram() + + def monk_display_name(ws_name, ndims): + return f"full {ws_name}: {ndims}" + + histogram.plot_display_name_callback = monk_display_name + + # clear mantid workspace + mtd.clear() + + workspace = CreateMDHistoWorkspace( + Dimensionality=4, + Extents="-3,3,-10,10,-20,20,-30,30", + SignalInput=range(0, 10000), + ErrorInput=range(0, 10000), + NumberOfBins="10,10,10,10", + Names="Dim1,Dim2,Dim3,Dim4", + Units="EnergyT,MomentumT,Other,Extra", + ) + + intensity_min = -10.34 + intensity_max = 12.09 + histogram.histogram_parameters.dimensions.intensity_min.setText(str(intensity_min)) + histogram.histogram_parameters.dimensions.intensity_max.setText(str(intensity_max)) + + data = histogram.get_plot_data(workspace, 4) + assert data[0] == "full workspace: 4" + assert data[1]["min"] == intensity_min + assert data[1]["max"] == intensity_max + + qtbot.wait(100) + + +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = None + logarithmic_intensity = False + """ + ], + indirect=True, +) +def test_plot_data_none(qtbot, user_conf_file, monkeypatch): + """Test plot inputs""" + + # mock plot info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + histogram = Histogram() + + # clear mantid workspace + mtd.clear() + + workspace = CreateMDHistoWorkspace( + Dimensionality=4, + Extents="-3,3,-10,10,-20,20,-30,30", + SignalInput=range(0, 10000), + ErrorInput=range(0, 10000), + NumberOfBins="10,10,10,10", + Names="Dim1,Dim2,Dim3,Dim4", + Units="EnergyT,MomentumT,Other,Extra", + ) + + intensity_max = 12.09 + + histogram.histogram_parameters.dimensions.intensity_max.setText(str(intensity_max)) + + data = histogram.get_plot_data(workspace, 4) + assert data[0] is None + assert data[1]["min"] is None + assert data[1]["max"] == intensity_max + + qtbot.wait(100) From 314380752183536a946d188b4bb177b6e095bee1 Mon Sep 17 00:00:00 2001 From: Maria Patrou Date: Mon, 21 Aug 2023 12:28:42 -0400 Subject: [PATCH 08/10] test_plots updates --- tests/views/test_plots.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/views/test_plots.py b/tests/views/test_plots.py index 3d57a7d8..9fdb2c69 100644 --- a/tests/views/test_plots.py +++ b/tests/views/test_plots.py @@ -302,7 +302,7 @@ def test_plot4d(qtbot, user_conf_file, monkeypatch): indirect=True, ) def test_plot4d_invalid_scale(qtbot, user_conf_file, monkeypatch): - """Test for 4D plot with intensities""" + """Test for 4D plot with invalid intensities""" # mock get_oncat_url, client_id and use_notes info monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) @@ -325,7 +325,7 @@ def test_plot4d_invalid_scale(qtbot, user_conf_file, monkeypatch): title = None view = do_default_plot(workspace, 4, title, {"min": intensity_min, "max": intensity_max}) # mantid plot updates user values if invalid are passed - assert view.data_view.colorbar.cmin_value == 0.0001 + assert view.data_view.colorbar.cmin_value != intensity_min assert view.data_view.colorbar.cmax_value == intensity_max assert view.data_view.colorbar.norm.currentText() == "Log" @@ -382,7 +382,7 @@ def test_plot5d(qtbot, user_conf_file, monkeypatch): indirect=True, ) def test_plot_data_name_only(qtbot, user_conf_file, monkeypatch): - """Test plot inputs""" + """Test plot inputs with name_only title""" # mock get_oncat_url, client_id and use_notes info monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) @@ -420,7 +420,7 @@ def test_plot_data_name_only(qtbot, user_conf_file, monkeypatch): indirect=True, ) def test_plot_data_full(qtbot, user_conf_file, monkeypatch): - """Test plot inputs""" + """Test plot inputs with a customized full title and intensitites""" # mock plot info monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) @@ -469,7 +469,7 @@ def monk_display_name(ws_name, ndims): indirect=True, ) def test_plot_data_none(qtbot, user_conf_file, monkeypatch): - """Test plot inputs""" + """Test plot inputs with no title and one intensity""" # mock plot info monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) From 14785660dfd8284fbf090043195a79de0168fa88 Mon Sep 17 00:00:00 2001 From: Maria Patrou Date: Mon, 21 Aug 2023 13:12:50 -0400 Subject: [PATCH 09/10] test update and configuration template description --- src/shiver/configuration_template.ini | 7 +++---- tests/views/test_plots.py | 6 ++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/shiver/configuration_template.ini b/src/shiver/configuration_template.ini index c307484e..e0a01093 100644 --- a/src/shiver/configuration_template.ini +++ b/src/shiver/configuration_template.ini @@ -1,16 +1,15 @@ [generate_tab.oncat] #url to oncat portal oncat_url = https://oncat.ornl.gov -#client id for on cat -# NOTE: the client ID is unique for Shiver +#client id for on cat; it is unique for Shiver client_id = 99025bb3-ce06-4f4b-bcf2-36ebf925cd1d -#boolean value: True or False for oncat files -#the flag indicates that the names of the datasets are stored in the Notes/Comments +#the flag (bool: True/False) indicates the location of the names of the datasets (notes/comments vs. sequence name) use_notes = False [main_tab.plot] #options: full prints dimension data, name_only, workspace title, None: no title is printed in plots title = True +#the flag (bool: True/False) indicates the plot scale (logarithmic or not) logarithmic_intensity = False [global.other] diff --git a/tests/views/test_plots.py b/tests/views/test_plots.py index 9fdb2c69..70e5cccf 100644 --- a/tests/views/test_plots.py +++ b/tests/views/test_plots.py @@ -321,16 +321,14 @@ def test_plot4d_invalid_scale(qtbot, user_conf_file, monkeypatch): ) intensity_min = -10.34 - intensity_max = 12.09 + intensity_max = 12.1 title = None view = do_default_plot(workspace, 4, title, {"min": intensity_min, "max": intensity_max}) + qtbot.wait(200) # mantid plot updates user values if invalid are passed assert view.data_view.colorbar.cmin_value != intensity_min - assert view.data_view.colorbar.cmax_value == intensity_max assert view.data_view.colorbar.norm.currentText() == "Log" - qtbot.wait(500) - @pytest.mark.parametrize( "user_conf_file", From e73eacff6521e8989f1abd7d1d8147f078915fa1 Mon Sep 17 00:00:00 2001 From: Maria Patrou Date: Mon, 21 Aug 2023 14:48:11 -0400 Subject: [PATCH 10/10] conf test for application startup added --- tests/models/test_configuration.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/models/test_configuration.py b/tests/models/test_configuration.py index 0162f124..12573514 100644 --- a/tests/models/test_configuration.py +++ b/tests/models/test_configuration.py @@ -4,6 +4,7 @@ from pathlib import Path import pytest +from shiver import main from shiver.configuration import Configuration, get_data @@ -177,3 +178,30 @@ def test_get_data_invalid(monkeypatch, user_conf_file): assert len(get_data("generate_tab.oncat", "")) == 3 # field assert get_data("generate_tab.oncat", "field_not_here") is None + + +@pytest.mark.parametrize( + "user_conf_file", + [ + """ + [main_tab.plot] + title = name_only + logarithmic_intensity = False + """ + ], + indirect=True, +) +def test_conf_init_invalid(capsys, user_conf_file, monkeypatch): + """Test starting the app with invalid configuration""" + + # mock conf info + monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file) + + def mock_is_valid(self): # pylint: disable=unused-argument + return False + + monkeypatch.setattr("shiver.configuration.Configuration.is_valid", mock_is_valid) + + main() + captured = capsys.readouterr() + assert captured[0].startswith("Error with configuration settings!")