From 692e2047a8acca11b316951ea6ea8cff718fa5cb Mon Sep 17 00:00:00 2001 From: Dagonite Date: Fri, 12 Nov 2021 16:34:53 +0000 Subject: [PATCH 01/10] Fix docstrings and use pythonic syntax --- .../reduction_viewer/views/run_summary.py | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/autoreduce_frontend/reduction_viewer/views/run_summary.py b/autoreduce_frontend/reduction_viewer/views/run_summary.py index 22e02f576..119b882d8 100644 --- a/autoreduce_frontend/reduction_viewer/views/run_summary.py +++ b/autoreduce_frontend/reduction_viewer/views/run_summary.py @@ -1,9 +1,11 @@ +# pylint:disable=no-member,too-many-locals,broad-except import logging + from django.shortcuts import redirect from django.urls import reverse + from autoreduce_db.reduction_viewer.models import ReductionRun from autoreduce_frontend.autoreduce_webapp.view_utils import (check_permissions, login_and_uows_valid, render_with) - from autoreduce_frontend.plotting.plot_handler import PlotHandler from autoreduce_frontend.reduction_viewer.views.common import get_arguments_from_file, prepare_arguments_for_render from autoreduce_frontend.reduction_viewer.view_utils import (get_interactive_plot_data, get_navigation_runs, @@ -15,7 +17,8 @@ def redirect_run_does_not_exist(instrument_name, run_number, run_version): """ - Redirects to the runs:list page if the run does not exist, and shows which run was not found. + Redirects to the runs:list page if the run does not exist, and shows which + run was not found. Args: instrument_name: The instrument name of the run. @@ -26,7 +29,6 @@ def redirect_run_does_not_exist(instrument_name, run_number, run_version): reverse("runs:list", kwargs={'instrument': instrument_name}), run_number, run_version)) -# pylint:disable=no-member,too-many-locals,broad-except @login_and_uows_valid @check_permissions @render_with('run_summary.html') @@ -45,7 +47,6 @@ def run_summary(request, instrument_name=None, run_number=None, run_version=0): @login_and_uows_valid @check_permissions @render_with('run_summary.html') -# pylint:disable=no-member,too-many-locals,broad-except,invalid-name def run_summary_batch_run(request, instrument_name=None, pk=None, run_version=0): """Gathers the context and renders a run's summary""" history = ReductionRun.objects.filter(instrument__name=instrument_name, pk=pk).order_by( @@ -65,7 +66,7 @@ def run_summary_run(request, history, instrument_name=None, run_version=0, run_n started_by = started_by_id_to_name(run.started_by) - # Run status value of "s" means the run is skipped + # Run status value of 's' means the run is skipped is_skipped = run.status.value == "s" is_rerun = len(history) > 1 @@ -73,26 +74,26 @@ def run_summary_run(request, history, instrument_name=None, run_version=0, run_n reduction_location = "" if location_list: reduction_location = location_list[0].file_path - if reduction_location and '\\' in reduction_location: + if '\\' in reduction_location: reduction_location = reduction_location.replace('\\', '/') - path_type = request.GET.get("path_type", "linux") # defaults to Linux + path_type = request.GET.get("path_type", "linux") # Defaults to Linux if path_type == "linux": data_location = "\n".join( - [windows_to_linux_path(data_location.file_path) for data_location in run.data_location.all()]) + windows_to_linux_path(data_location.file_path) for data_location in run.data_location.all()) elif path_type == "windows": data_location = "\n".join( - [linux_to_windows_path(data_location.file_path) for data_location in run.data_location.all()]) + linux_to_windows_path(data_location.file_path) for data_location in run.data_location.all()) data_analysis_link_url = make_data_analysis_url(reduction_location) if reduction_location else "" rb_number = run.experiment.reference_number standard_vars, advanced_vars, variable_help = prepare_arguments_for_render(run.arguments, run.instrument.name) - default_standard_variables, _, __ = get_arguments_from_file(run.instrument.name) + default_standard_variables, *_ = get_arguments_from_file(run.instrument.name) - # picks the unique identifier for a run - for batch runs, it's the pk, - # and for normal runs it's just the run number + # Picks the unique identifier for a run - for batch runs, it's the pk, and + # for normal runs it's just the run number run_unique_id = run.pk if run.batch_run else run.run_number - runs = ",".join([str(r.run_number) for r in run.run_numbers.all()]) + runs = ",".join(str(r.run_number) for r in run.run_numbers.all()) page_type = request.GET.get('sort', '-run_number') next_run, previous_run, newest_run, oldest_run = get_navigation_runs(instrument_name, run, page_type) @@ -131,6 +132,7 @@ def run_summary_run(request, history, instrument_name=None, run_version=0, run_n plot_handler = PlotHandler(data_filepath=run.data_location.first().file_path, server_dir=reduction_location, rb_number=rb_number) + local_plot_locs, server_plot_locs = plot_handler.get_plot_file() if local_plot_locs: context_dictionary['static_plots'] = [ @@ -138,7 +140,7 @@ def run_summary_run(request, history, instrument_name=None, run_version=0, run_n ] context_dictionary['interactive_plots'] = get_interactive_plot_data(server_plot_locs) - except Exception as exception: # pylint: disable=broad-except + except Exception as exception: # Lack of plot images is recoverable - we shouldn't stop the whole # page rendering if something is wrong with the plot images - but # display an error message From 4ee2cb6542c9fe48b7f5e70e50f768b11d45043b Mon Sep 17 00:00:00 2001 From: Dagonite Date: Fri, 12 Nov 2021 17:28:42 +0000 Subject: [PATCH 02/10] Fix docstrings and formatting --- autoreduce_frontend/autoreduce_webapp/views.py | 5 ++--- autoreduce_frontend/reduction_viewer/views/runs_list.py | 5 ++++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/autoreduce_frontend/autoreduce_webapp/views.py b/autoreduce_frontend/autoreduce_webapp/views.py index 4dcc3eb07..2c63b3e1e 100644 --- a/autoreduce_frontend/autoreduce_webapp/views.py +++ b/autoreduce_frontend/autoreduce_webapp/views.py @@ -1,9 +1,9 @@ # ############################################################################### # # Autoreduction Repository : https://github.com/autoreduction/autoreduce # -# Copyright © 2019 ISIS Rutherford Appleton Laboratory UKRI +# Copyright © 2021 ISIS Rutherford Appleton Laboratory UKRI # SPDX - License - Identifier: GPL-3.0-or-later -# ############################################################################### # +# ############################################################################ # """Handle page responses for the web app.""" # pylint: disable=unused-argument,bare-except,no-member from django.http import HttpRequest @@ -18,7 +18,6 @@ def render_error(request: HttpRequest, message: str): Args: request: The original sent request. - message: The message that will be displayed. Return: diff --git a/autoreduce_frontend/reduction_viewer/views/runs_list.py b/autoreduce_frontend/reduction_viewer/views/runs_list.py index 4b351925f..badee91ca 100644 --- a/autoreduce_frontend/reduction_viewer/views/runs_list.py +++ b/autoreduce_frontend/reduction_viewer/views/runs_list.py @@ -1,3 +1,4 @@ +# pylint:disable=no-member,unused-argument,too-many-locals,broad-except import traceback import logging @@ -16,7 +17,6 @@ @login_and_uows_valid @check_permissions @render_with('runs_list.html') -# pylint:disable=no-member,unused-argument,too-many-locals,broad-except def runs_list(request, instrument=None): """Render instrument summary.""" try: @@ -78,16 +78,19 @@ def runs_list(request, instrument=None): if filter_by == 'experiment': experiments_and_runs = {} + experiments = Experiment.objects.filter(reduction_runs__instrument=instrument_obj). \ order_by('-reference_number').distinct() for experiment in experiments: associated_runs = runs.filter(experiment=experiment). \ order_by('-created') experiments_and_runs[experiment] = associated_runs + experiment_table = ExperimentTable(experiments) RequestConfig(request, paginate={"per_page": 10}).configure(experiment_table) context_dictionary['experiments'] = experiments_and_runs context_dictionary['experiment_table'] = experiment_table + elif filter_by == 'batch_runs': runs = ReductionRun.objects.only('status', 'last_updated', 'run_version', 'run_description').filter(instrument=instrument_obj, batch_run=True) From 0cc8e2728809dc08b477a10fd517c0e8ac058690 Mon Sep 17 00:00:00 2001 From: Dagonite Date: Fri, 12 Nov 2021 17:58:36 +0000 Subject: [PATCH 03/10] Fix docstrings and type hinting --- .../selenium_tests/pages/runs_list_page.py | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/autoreduce_frontend/selenium_tests/pages/runs_list_page.py b/autoreduce_frontend/selenium_tests/pages/runs_list_page.py index 4111f9ab9..8f48c1e3e 100644 --- a/autoreduce_frontend/selenium_tests/pages/runs_list_page.py +++ b/autoreduce_frontend/selenium_tests/pages/runs_list_page.py @@ -3,7 +3,7 @@ # # Copyright © 2021 ISIS Rutherford Appleton Laboratory UKRI # SPDX - License - Identifier: GPL-3.0-or-later -# ############################################################################### # +# ############################################################################ # """Module for the instrument summary page model.""" from typing import List @@ -63,7 +63,6 @@ def click_run(self, run_number: int, version: int = 0) -> RunSummaryPage: Args: run_number: Run number of the link to click. - version: Version of the run to click. Returns: @@ -108,26 +107,21 @@ def click_btn_by_title(self, title: str) -> None: raise NoSuchElementException def click_next_page_button(self) -> None: - """ - Click next page pagination button - """ + """Click next page pagination button.""" btn = self.driver.find_element_by_xpath("//li[@class='next page-item']/a") btn.click() def click_prev_page_button(self) -> None: - """ - Click previous page pagination button - """ + """Click previous page pagination button.""" btn = self.driver.find_element_by_xpath("//li[@class='prev page-item']/a") btn.click() - def update_filter(self, filter_name, value): + def update_filter(self, filter_name: str, value: str) -> None: """ Select a valid filter option. Args: filter_name: The name of the filter type being updated. - value: The new value of the given filter. """ Select(self.driver.find_element_by_id(filter_name)).select_by_visible_text(value) @@ -138,6 +132,9 @@ def click_apply_filters(self) -> None: btn.click() @property - def top_alert_message_text(self): - """Finds and returns the alert message shown at the top of the runs list page""" + def top_alert_message_text(self) -> str: + """ + Find and return the alert message shown at the top of the runs list + page. + """ return self.driver.find_element_by_id("top-alert-message").text From d0a56a7014f131b85539d5bd600bff7c2f1de305 Mon Sep 17 00:00:00 2001 From: Dagonite Date: Fri, 12 Nov 2021 18:25:25 +0000 Subject: [PATCH 04/10] Fix test logic --- .../tests/pages/test_runs_list.py | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/autoreduce_frontend/selenium_tests/tests/pages/test_runs_list.py b/autoreduce_frontend/selenium_tests/tests/pages/test_runs_list.py index 861ca09f7..78c443d0e 100644 --- a/autoreduce_frontend/selenium_tests/tests/pages/test_runs_list.py +++ b/autoreduce_frontend/selenium_tests/tests/pages/test_runs_list.py @@ -125,30 +125,30 @@ def test_run_navigation_btns(self): Test that the run navigation buttons work. Begin from the fifth run in the fixture. """ - for nav in ("newest", "next", "previous"): - self.page.launch() - runs = self.page.get_run_numbers_from_table() - fifth_run = runs[4] - run_summary_page = self.page.click_run(fifth_run) - assert run_summary_page.title_text() == "Reduction Job #100005" - time.sleep(3) - run_summary_page.click_btn_by_id(nav) - time.sleep(3) - if nav == "next": - assert run_summary_page.title_text() == "Reduction Job #100006" - elif nav == "previous": - assert run_summary_page.title_text() == "Reduction Job #100004" - elif nav == "newest": - assert run_summary_page.title_text() == "Reduction Job #100009" + self.page.launch() + runs = self.page.get_run_numbers_from_table() + fifth_run = runs[4] + + run_summary_page = self.page.click_run(fifth_run) + assert run_summary_page.title_text() == "Reduction Job #100005" + + run_summary_page.click_btn_by_id("next") + assert run_summary_page.title_text() == "Reduction Job #100006" + + run_summary_page.click_btn_by_id("previous") + assert run_summary_page.title_text() == "Reduction Job #100005" + + run_summary_page.click_btn_by_id("newest") + assert run_summary_page.title_text() == "Reduction Job #100009" def test_disabled_btns(self): """ Test that the run summary navigation buttons are disabled depending on a runs' recency. - The latest run should have the `Newest` and `Next` buttons disabled. + The latest run should have the 'Newest' and 'Next' buttons disabled. - The oldest run should have the `Previous` button disabled. + The oldest run should have the 'Previous' button disabled. Note: This test assumes that the runs are being sorted by number. From e33a058ad27af6d43895e2a1a7357eac12df1fcd Mon Sep 17 00:00:00 2001 From: Dagonite Date: Mon, 15 Nov 2021 18:48:48 +0000 Subject: [PATCH 05/10] Fix docstrings and formatting --- .../reduction_viewer/view_utils.py | 51 ++++++------- .../reduction_viewer/views/common.py | 75 +++++++++++-------- .../reduction_viewer/views/run_summary.py | 18 +++-- .../reduction_viewer/views/runs_list.py | 18 +++-- .../selenium_tests/configuration.py | 47 +++++------- autoreduce_frontend/templates/runs_list.html | 4 +- 6 files changed, 109 insertions(+), 104 deletions(-) diff --git a/autoreduce_frontend/reduction_viewer/view_utils.py b/autoreduce_frontend/reduction_viewer/view_utils.py index e232bf65e..f4b79ef96 100644 --- a/autoreduce_frontend/reduction_viewer/view_utils.py +++ b/autoreduce_frontend/reduction_viewer/view_utils.py @@ -1,9 +1,9 @@ -# ############################################################################### # +# ############################################################################ # # Autoreduction Repository : https://github.com/autoreduction/autoreduce # # Copyright © 2021 ISIS Rutherford Appleton Laboratory UKRI # SPDX - License - Identifier: GPL-3.0-or-later -# ############################################################################### # +# ############################################################################ # """Utility functions for the view of django models.""" # pylint:disable=no-member import functools @@ -14,10 +14,11 @@ from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist from django.utils.http import url_has_allowed_host_and_scheme + from autoreduce_db.reduction_viewer.models import Instrument, ReductionRun from autoreduce_qp.queue_processor.reduction.service import ReductionScript from autoreduce_frontend.autoreduce_webapp.settings import DATA_ANALYSIS_BASE_URL -from autoreduce_frontend.autoreduce_webapp.settings import (ALLOWED_HOSTS, UOWS_LOGIN_URL) +from autoreduce_frontend.autoreduce_webapp.settings import ALLOWED_HOSTS, UOWS_LOGIN_URL from autoreduce_frontend.autoreduce_webapp.templatetags.colour_table_row import colour_table_row LOGGER = logging.getLogger(__package__) @@ -59,25 +60,34 @@ def get_interactive_plot_data(plot_locations): def make_data_analysis_url(reduction_location: str) -> str: """ - Makes a URL for the data.analysis website that will open the location of the + Make a URL for the data.analysis website that will open the location of the data. """ if "/instrument/" in reduction_location: return DATA_ANALYSIS_BASE_URL + reduction_location.split("/instrument/")[1] + return "" def windows_to_linux_path(path: str) -> str: - """Convert Windows path to Linux path.""" - # '\\isis\inst$\' maps to '/isis/' + r""" + Convert Windows path to Linux path. + + Note: + '\\isis\inst$\' maps to '/isis/' + """ path = path.replace(r'\\isis\inst$' + '\\', '/isis/') path = path.replace('\\', '/') return path def linux_to_windows_path(path: str) -> str: - """Convert Linux path to Windows path.""" - # '\\isis\inst$\' maps to '/isis/' + r""" + Convert Linux path to Windows path. + + Note: + '\\isis\inst$\' maps to '/isis/' + """ path = path.replace('/isis/', r'\\isis\inst$' + '\\') path = path.replace('/', '\\') return path @@ -92,13 +102,8 @@ def started_by_id_to_name(started_by_id=None): if not started by a user. Returns: - If started by a valid user, return '[forename] [surname]'. - - If started automatically, return 'Autoreducton service'. - - If started manually, return 'Development team'. - - Otherwise, return None. + A string of the name of the user or team that submitted an Autoreduction + run. """ if started_by_id is None or started_by_id < -1: return None @@ -136,9 +141,7 @@ def make_return_url(request, next_url): def order_runs(sort_by: str, runs: ReductionRun.objects): - """ - Sort a queryset of runs based on the passed GET sort_by param - """ + """Sort a queryset of runs based on the passed GET sort_by param.""" if sort_by == "-run_number": runs = runs.order_by('-run_numbers__run_number', '-run_version') elif sort_by == "run_number": @@ -153,15 +156,9 @@ def order_runs(sort_by: str, runs: ReductionRun.objects): return runs -# pylint:disable=no-method-argument -def data_status(status): - """Function to add text-(status) class to status column for formatting - - Returns: - "text- " concatonated with status fetched from record for formatting with colour_table_row - - """ - return "text-" + colour_table_row(status) + " run-status" +def data_status(status: str) -> str: + """Add text-(status) class to status column for formatting.""" + return f"text-{colour_table_row(status)} run-status" def get_navigation_runs(instrument_name: str, run: ReductionRun, page_type: str) -> Tuple[ReductionRun]: diff --git a/autoreduce_frontend/reduction_viewer/views/common.py b/autoreduce_frontend/reduction_viewer/views/common.py index d14cdc9e9..32df2f088 100644 --- a/autoreduce_frontend/reduction_viewer/views/common.py +++ b/autoreduce_frontend/reduction_viewer/views/common.py @@ -1,3 +1,11 @@ +# ############################################################################ # +# Autoreduction Repository : +# https://github.com/ISISScientificComputing/autoreduce +# +# Copyright © 2021 ISIS Rutherford Appleton Laboratory UKRI +# SPDX - License - Identifier: GPL-3.0-or-later +# ############################################################################ # +# pylint:disable=too-many-return-statements import base64 import itertools import json @@ -8,7 +16,8 @@ UNAUTHORIZED_MESSAGE = "User is not authorized to submit batch runs. Please contact the Autoreduce team "\ "at ISISREDUCE@stfc.ac.uk to request the permissions." # Holds the default value used when there is no value for the variable -# in the default variables dictionary. Stored in a parameter for re-use in tests. +# in the default variables dictionary. Stored in a parameter for re-use in +# tests. DEFAULT_WHEN_NO_VALUE = "" @@ -25,9 +34,10 @@ def _combine_dicts(current: dict, default: dict): final = {} for name in itertools.chain(current.keys(), default.keys()): - # the default value for argument, also used when the variable is missing from the current variables - # ideally there will always be a default for each variable name, but - # if the variable is missing from the default dictionary, then just default to empty string + # The default value for argument, also used when the variable is missing + # from the current variables ideally there will always be a default for + # each variable name, but if the variable is missing from the default + # dictionary, then just default to empty string default_value = default.get(name, DEFAULT_WHEN_NO_VALUE) final[name] = {"current": current.get(name, default_value), "default": default_value} @@ -43,7 +53,8 @@ def unpack_arguments(arguments: dict) -> Tuple[dict, dict, dict]: arguments: The arguments dictionary to unpack. Returns: - A tuple containing the standard variables, advanced variables, and variable help. + A tuple containing the standard variables, advanced variables, and + variable help. """ standard_arguments = arguments.get("standard_vars", {}) advanced_arguments = arguments.get("advanced_vars", {}) @@ -60,8 +71,8 @@ def get_arguments_from_file(instrument: str) -> Tuple[dict, dict, dict]: Raises: FileNotFoundError: If the instrument's reduce_vars file is not found. - ImportError: If the instrument's reduce_vars file contains an import error. - SyntaxError: If the instrument's reduce_vars file contains a syntax error. + ImportError: If the reduce_vars file contains an import error. + SyntaxError: If the reduce_vars file contains a syntax error. """ default_variables = VariableUtils.get_default_variables(instrument) default_standard_variables, default_advanced_variables, variable_help = unpack_arguments(default_variables) @@ -70,7 +81,8 @@ def get_arguments_from_file(instrument: str) -> Tuple[dict, dict, dict]: def prepare_arguments_for_render(arguments: ReductionArguments, instrument: str) -> Tuple[dict, dict, dict]: """ - Converts the arguments into a dictionary containing their "current" and "default" values. + Converts the arguments into a dictionary containing their "current" and + "default" values. Used to render the form in the webapp (with values from "current"), and provide the defaults for resetting (with values from "default"). @@ -80,7 +92,8 @@ def prepare_arguments_for_render(arguments: ReductionArguments, instrument: str) instrument: The instrument to get the default variables for. Returns: - A dictionary containing the arguments and their current and default values. + A dictionary containing the arguments and their current and default + values. """ vars_kwargs = arguments.as_dict() standard_vars = vars_kwargs.get("standard_vars", {}) @@ -95,55 +108,54 @@ def prepare_arguments_for_render(arguments: ReductionArguments, instrument: str) def decode_b64(value: str): - """ - Decodes the base64 representation back to utf-8 string. - """ + """Decodes the base64 representation back to utf-8 string.""" return base64.urlsafe_b64decode(value).decode("utf-8") -# pylint:disable=too-many-return-statements def convert_to_python_type(value: str): """ - Converts the string sent by the POST request to a real Python type that can be serialized by JSON + Converts the string sent by the POST request to a real Python type that can + be serialized by JSON. Args: - value: The string value to convert + value: The string value to convert. Returns: - The converted value + The converted value. """ try: - # json can directly load str/int/floats and lists of them + # JSON can directly load str/int/floats and lists of them return json.loads(value) except json.JSONDecodeError: - if value.lower() == "none" or value.lower() == "null": + lowered_value = value.lower() + if lowered_value in ("none", "null"): return None - elif value.lower() == "true": + if lowered_value == "true": return True - elif value.lower() == "false": + if lowered_value == "false": return False - elif "," in value and "[" not in value and "]" not in value: + if "," in value and "[" not in value and "]" not in value: return convert_to_python_type(f"[{value}]") - elif "'" in value: + if "'" in value: return convert_to_python_type(value.replace("'", '"')) - else: - return value + + return value def make_reduction_arguments(post_arguments: dict, instrument: str) -> dict: """ - Given new variables from the POST request and the default variables from reduce_vars.py - create a dictionary of the new variables + Given new variables from the POST request and the default variables from + reduce_vars.py create a dictionary of the new variables Args: - post_arguments: The new variables to be created - default_variables: The default variables + post_arguments: The new variables to be created. + default_variables: The default variables. Returns: - The new variables as a dict + The new variables as a dict. Raises: - ValueError if any variable values exceed the allowed maximum + ValueError: If any variable values exceed the allowed maximum. """ defaults = VariableUtils.get_default_variables(instrument) @@ -161,9 +173,10 @@ def make_reduction_arguments(post_arguments: dict, instrument: str) -> dict: if name is not None: name = decode_b64(name) - # skips variables that have been removed from the defaults + # Skips variables that have been removed from the defaults if name not in defaults[dict_key]: continue defaults[dict_key][name] = convert_to_python_type(value) + return defaults diff --git a/autoreduce_frontend/reduction_viewer/views/run_summary.py b/autoreduce_frontend/reduction_viewer/views/run_summary.py index 119b882d8..2c99c95ac 100644 --- a/autoreduce_frontend/reduction_viewer/views/run_summary.py +++ b/autoreduce_frontend/reduction_viewer/views/run_summary.py @@ -1,3 +1,10 @@ +# ############################################################################ # +# Autoreduction Repository : +# https://github.com/ISISScientificComputing/autoreduce +# +# Copyright © 2021 ISIS Rutherford Appleton Laboratory UKRI +# SPDX - License - Identifier: GPL-3.0-or-later +# ############################################################################ # # pylint:disable=no-member,too-many-locals,broad-except import logging @@ -5,7 +12,7 @@ from django.urls import reverse from autoreduce_db.reduction_viewer.models import ReductionRun -from autoreduce_frontend.autoreduce_webapp.view_utils import (check_permissions, login_and_uows_valid, render_with) +from autoreduce_frontend.autoreduce_webapp.view_utils import check_permissions, login_and_uows_valid, render_with from autoreduce_frontend.plotting.plot_handler import PlotHandler from autoreduce_frontend.reduction_viewer.views.common import get_arguments_from_file, prepare_arguments_for_render from autoreduce_frontend.reduction_viewer.view_utils import (get_interactive_plot_data, get_navigation_runs, @@ -17,7 +24,7 @@ def redirect_run_does_not_exist(instrument_name, run_number, run_version): """ - Redirects to the runs:list page if the run does not exist, and shows which + Redirect to the runs:list page if the run does not exist, and shows which run was not found. Args: @@ -48,7 +55,7 @@ def run_summary(request, instrument_name=None, run_number=None, run_version=0): @check_permissions @render_with('run_summary.html') def run_summary_batch_run(request, instrument_name=None, pk=None, run_version=0): - """Gathers the context and renders a run's summary""" + """Gather the context and renders a run's summary.""" history = ReductionRun.objects.filter(instrument__name=instrument_name, pk=pk).order_by( '-run_version').select_related('status').select_related('experiment').select_related('instrument') if len(history) == 0: @@ -58,7 +65,7 @@ def run_summary_batch_run(request, instrument_name=None, pk=None, run_version=0) def run_summary_run(request, history, instrument_name=None, run_version=0, run_number=0): - """Gathers the context and renders a run's summary""" + """Gather the context and renders a run's summary.""" try: run = next(run for run in history if run.run_version == int(run_version)) except StopIteration: @@ -132,14 +139,13 @@ def run_summary_run(request, history, instrument_name=None, run_version=0, run_n plot_handler = PlotHandler(data_filepath=run.data_location.first().file_path, server_dir=reduction_location, rb_number=rb_number) - local_plot_locs, server_plot_locs = plot_handler.get_plot_file() if local_plot_locs: context_dictionary['static_plots'] = [ location for location in local_plot_locs if not location.endswith(".json") ] - context_dictionary['interactive_plots'] = get_interactive_plot_data(server_plot_locs) + except Exception as exception: # Lack of plot images is recoverable - we shouldn't stop the whole # page rendering if something is wrong with the plot images - but diff --git a/autoreduce_frontend/reduction_viewer/views/runs_list.py b/autoreduce_frontend/reduction_viewer/views/runs_list.py index badee91ca..572ade71e 100644 --- a/autoreduce_frontend/reduction_viewer/views/runs_list.py +++ b/autoreduce_frontend/reduction_viewer/views/runs_list.py @@ -1,11 +1,18 @@ +# ############################################################################ # +# Autoreduction Repository : +# https://github.com/ISISScientificComputing/autoreduce +# +# Copyright © 2021 ISIS Rutherford Appleton Laboratory UKRI +# SPDX - License - Identifier: GPL-3.0-or-later +# ############################################################################ # # pylint:disable=no-member,unused-argument,too-many-locals,broad-except import traceback import logging -from autoreduce_db.reduction_viewer.models import Experiment, Instrument, ReductionRun, Status -from autoreduce_qp.queue_processor.variable_utils import VariableUtils from django_tables2 import RequestConfig +from autoreduce_db.reduction_viewer.models import Experiment, Instrument, ReductionRun, Status +from autoreduce_qp.queue_processor.variable_utils import VariableUtils from autoreduce_frontend.autoreduce_webapp.view_utils import check_permissions, login_and_uows_valid, render_with from autoreduce_frontend.reduction_viewer.view_utils import order_runs from autoreduce_frontend.reduction_viewer.tables import ExperimentTable, ReductionRunTable @@ -35,7 +42,6 @@ def runs_list(request, instrument=None): first_instrument_run = runs.filter(batch_run=False).first() runs = order_runs(sort_by=sort_by, runs=runs) - run_table = ReductionRunTable(runs) RequestConfig(request, paginate={"per_page": 10}).configure(run_table) @@ -48,14 +54,13 @@ def runs_list(request, instrument=None): return {'message': "No runs found for instrument."} current_variables = {} + error_reason = "" try: current_variables.update(VariableUtils.get_default_variables(instrument_obj.name, raise_exc=True)) except FileNotFoundError: error_reason = "reduce_vars.py is missing for this instrument" except (ImportError, SyntaxError): error_reason = "reduce_vars.py has an import or syntax error" - else: - error_reason = "" context_dictionary = { 'instrument': instrument_obj, @@ -82,8 +87,7 @@ def runs_list(request, instrument=None): experiments = Experiment.objects.filter(reduction_runs__instrument=instrument_obj). \ order_by('-reference_number').distinct() for experiment in experiments: - associated_runs = runs.filter(experiment=experiment). \ - order_by('-created') + associated_runs = runs.filter(experiment=experiment).order_by('-created') experiments_and_runs[experiment] = associated_runs experiment_table = ExperimentTable(experiments) diff --git a/autoreduce_frontend/selenium_tests/configuration.py b/autoreduce_frontend/selenium_tests/configuration.py index b8f021edf..f099a93ca 100644 --- a/autoreduce_frontend/selenium_tests/configuration.py +++ b/autoreduce_frontend/selenium_tests/configuration.py @@ -1,12 +1,12 @@ -# ############################################################################### # +# ############################################################################ # # Autoreduction Repository : https://github.com/autoreduction/autoreduce # # Copyright © 2020 ISIS Rutherford Appleton Laboratory UKRI # SPDX - License - Identifier: GPL-3.0-or-later -# ############################################################################### # +# ############################################################################ # """ -Module containing configuration functions. Configuration file must be used as a thread safe way to -share values between xdist workers does not currently exist. +Module containing configuration functions. Configuration file must be used as a +thread safe way to share values between xdist workers does not currently exist. """ import json import sys @@ -28,8 +28,8 @@ def store_original_config(): """ - Make a copy of the config file to a temporary file. This prevents arguments given to the test - entrypoint being persisted to the config file. + Make a copy of the config file to a temporary file. This prevents arguments + given to the test entrypoint being persisted to the config file. """ try: copyfile(SELENIUM_CONFIG, TEMP_SELENIUM_CONFIG) @@ -38,36 +38,29 @@ def store_original_config(): def get_url(): - """ - Returns the url to test against from the config - :return: (str) The url to test against from the config - """ - + """Returns the URL to test against from the config.""" return load_config_file()["url"] def is_headless(): - """ - Returns the headless boolean from the config - :return: (bool) The headless boolean from the config - """ + """Returns the headless boolean from the config.""" return load_config_file()["run_headless"] def set_url(url): """ - Set the url to test against in the config. IPs must be prefixed with http/https still - :param url: (str) The url to test against + Set the url to test against in the config. IPs must be prefixed with + 'http'/'https' still. """ config = load_config_file() config["url"] = url dump_to_config_file(config) -def set_headless(headless): +def set_headless(headless: bool): """ - Set the headless option in the config to decide whether or not to use a headless driver - :param headless: (bool) the headless bool option + Set the headless option in the config to decide whether or not to use a + headless driver. """ config = load_config_file() config["run_headless"] = headless @@ -76,8 +69,8 @@ def set_headless(headless): def cleanup_config(): """ - Copy the original values back to the original config file, so as not to persist arguments given - to test runner + Copy the original values back to the original config file, so as not to + persist arguments given to test runner. """ copyfile(TEMP_SELENIUM_CONFIG, SELENIUM_CONFIG) if TEMP_SELENIUM_CONFIG.exists(): @@ -85,10 +78,7 @@ def cleanup_config(): def load_config_file(): - """ - Load the config file into a python dictionary - :return: (dict) The config file as a python dictionary - """ + """Load and return the config file into a python dictionary.""" try: with open(SELENIUM_CONFIG) as fle: return json.load(fle) @@ -97,9 +87,6 @@ def load_config_file(): def dump_to_config_file(config_dict): - """ - Dump the given dictionary to the config file - :param config_dict: (dict) the dictionary to be dumped. - """ + """Dump the given dictionary to the config file.""" with open(SELENIUM_CONFIG, "w") as fle: json.dump(config_dict, fle, indent=4) diff --git a/autoreduce_frontend/templates/runs_list.html b/autoreduce_frontend/templates/runs_list.html index 8d36f1660..c1b699bf8 100644 --- a/autoreduce_frontend/templates/runs_list.html +++ b/autoreduce_frontend/templates/runs_list.html @@ -86,6 +86,7 @@

{{ instrument_name }}



+
@@ -103,9 +104,6 @@

{{ instrument_name }}

- - -
From 3d1dbfb0e4c34af0122e8b54bbfa9b85eb8bfb33 Mon Sep 17 00:00:00 2001 From: Dagonite Date: Mon, 15 Nov 2021 18:49:19 +0000 Subject: [PATCH 06/10] Refactor multiple returns into dict --- .../templatetags/colour_table_row.py | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/autoreduce_frontend/autoreduce_webapp/templatetags/colour_table_row.py b/autoreduce_frontend/autoreduce_webapp/templatetags/colour_table_row.py index 02cf13fae..e6895b73b 100644 --- a/autoreduce_frontend/autoreduce_webapp/templatetags/colour_table_row.py +++ b/autoreduce_frontend/autoreduce_webapp/templatetags/colour_table_row.py @@ -1,31 +1,25 @@ -# ############################################################################### # +# ############################################################################ # # Autoreduction Repository : https://github.com/autoreduction/autoreduce # -# Copyright © 2019 ISIS Rutherford Appleton Laboratory UKRI +# Copyright © 2021 ISIS Rutherford Appleton Laboratory UKRI # SPDX - License - Identifier: GPL-3.0-or-later -# ############################################################################### # -""" -Handles colouring table rows -""" +# ############################################################################ # +# pylint:disable=invalid-name +"""Handles colouring table rows.""" from django.template import Library -# pylint:disable=invalid-name register = Library() +_STATUSES = { + "Error": "danger", + "Processing": "warning", + "Queued": "info", + "Completed": "success", + "Skipped": "dark", +} + @register.simple_tag def colour_table_row(status): - """ - Switch statement for defining table colouring - """ - if status == 'Error': - return 'danger' - if status == 'Processing': - return 'warning' - if status == 'Queued': - return 'info' - if status == 'Completed': - return 'success' - if status == 'Skipped': - return 'dark' - return status + """Switch statement for defining table colouring.""" + return _STATUSES.get(status, status) From 3643d35f3b4fd16418da0aa633815e300fcb6cfd Mon Sep 17 00:00:00 2001 From: Dagonite Date: Tue, 7 Dec 2021 16:46:34 +0000 Subject: [PATCH 07/10] Fix docstrings --- .../reduction_viewer/views/configure_new_runs.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/autoreduce_frontend/reduction_viewer/views/configure_new_runs.py b/autoreduce_frontend/reduction_viewer/views/configure_new_runs.py index e94f53bed..328c20ca9 100644 --- a/autoreduce_frontend/reduction_viewer/views/configure_new_runs.py +++ b/autoreduce_frontend/reduction_viewer/views/configure_new_runs.py @@ -16,23 +16,21 @@ @check_permissions @render_with('configure_new_runs.html') def configure_new_runs(request, instrument=None, start=0, experiment_reference=0): - """ - Handles request to view instrument variables - """ + """Handle request to view instrument variables.""" instrument_name = instrument if request.method == 'POST': return configure_new_runs_post(request, instrument_name) - else: - return configure_new_runs_get(instrument, start, experiment_reference) + return configure_new_runs_get(instrument, start, experiment_reference) def configure_new_runs_post(request, instrument_name): """ Submission to modify variables. Acts on POST request. - Depending on the parameters it either makes them for a run range (when start is given, end is optional) - or for experiment reference (when experiment_reference is given). + Depending on the parameters it either makes them for a run range (when start + is given, end is optional) or for experiment reference (when + experiment_reference is given). """ start = int(request.POST.get("run_start")) if request.POST.get("run_start", None) else None experiment_reference = int(request.POST.get("experiment_reference_number")) if request.POST.get( @@ -63,9 +61,7 @@ def update_or_create(instrument, arguments_json, kwargs): # pylint:disable=too-many-locals def configure_new_runs_get(instrument_name, start=0, experiment_reference=0): - """ - GET for the configure new runs page - """ + """GET for the configure new runs page.""" instrument = Instrument.objects.get(name__iexact=instrument_name) editing = (start > 0 or experiment_reference > 0) From 3b9f68362f71f8c4da28691e97a3b7edf8104a71 Mon Sep 17 00:00:00 2001 From: Dagonite Date: Thu, 23 Dec 2021 08:50:05 +0000 Subject: [PATCH 08/10] Fix comment --- autoreduce_frontend/reduction_viewer/views/run_summary.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autoreduce_frontend/reduction_viewer/views/run_summary.py b/autoreduce_frontend/reduction_viewer/views/run_summary.py index 2c99c95ac..27ea36453 100644 --- a/autoreduce_frontend/reduction_viewer/views/run_summary.py +++ b/autoreduce_frontend/reduction_viewer/views/run_summary.py @@ -97,7 +97,7 @@ def run_summary_run(request, history, instrument_name=None, run_version=0, run_n standard_vars, advanced_vars, variable_help = prepare_arguments_for_render(run.arguments, run.instrument.name) default_standard_variables, *_ = get_arguments_from_file(run.instrument.name) - # Picks the unique identifier for a run - for batch runs, it's the pk, and + # Picks the unique identifier for a run - for batch runs it's the pk, and # for normal runs it's just the run number run_unique_id = run.pk if run.batch_run else run.run_number runs = ",".join(str(r.run_number) for r in run.run_numbers.all()) From 6e99fb2a1f9b77ce5e06b543206cb80df06c9349 Mon Sep 17 00:00:00 2001 From: Dagonite Date: Fri, 24 Dec 2021 09:53:50 +0000 Subject: [PATCH 09/10] Fetch json data from api --- .../reduction_viewer/views/common.py | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/autoreduce_frontend/reduction_viewer/views/common.py b/autoreduce_frontend/reduction_viewer/views/common.py index 32df2f088..14822b94a 100644 --- a/autoreduce_frontend/reduction_viewer/views/common.py +++ b/autoreduce_frontend/reduction_viewer/views/common.py @@ -5,11 +5,15 @@ # Copyright © 2021 ISIS Rutherford Appleton Laboratory UKRI # SPDX - License - Identifier: GPL-3.0-or-later # ############################################################################ # -# pylint:disable=too-many-return-statements +# pylint:disable=too-many-return-statements,broad-except import base64 import itertools import json +import os from typing import Tuple + +import requests + from autoreduce_db.reduction_viewer.models import ReductionArguments from autoreduce_qp.queue_processor.variable_utils import VariableUtils @@ -17,7 +21,7 @@ "at ISISREDUCE@stfc.ac.uk to request the permissions." # Holds the default value used when there is no value for the variable # in the default variables dictionary. Stored in a parameter for re-use in -# tests. +# tests DEFAULT_WHEN_NO_VALUE = "" @@ -96,6 +100,8 @@ def prepare_arguments_for_render(arguments: ReductionArguments, instrument: str) values. """ vars_kwargs = arguments.as_dict() + fetch_api_urls(vars_kwargs) + standard_vars = vars_kwargs.get("standard_vars", {}) advanced_vars = vars_kwargs.get("advanced_vars", {}) @@ -107,6 +113,34 @@ def prepare_arguments_for_render(arguments: ReductionArguments, instrument: str) return final_standard, final_advanced, variable_help +def fetch_api_urls(vars_kwargs): + """Convert file URLs in vars_kwargs into API URL strings.""" + for category, headings in vars_kwargs.items(): + for heading, heading_value in headings.items(): + if "file" in heading.lower() and isinstance(heading_value, dict): + try: + vars_kwargs[category][heading]["all_files"] = {} + path = heading_value["url"].partition("master")[2] + url = "https://api.github.com/repos/mantidproject/scriptrepository/contents" + path + vars_kwargs[category][heading]["api"] = url + req = requests.get(url) + data = json.loads(req.content) + + for link in data: + file_name = link["name"] + url, _, default = link["download_url"].rpartition("/") + auth_token = os.environ.get("AUTOREDUCTION_GITHUB_AUTH_TOKEN", "") + req = requests.get(f"{url}/{default}", headers={"Authorization": f"Token {auth_token}"}) + value = req.text + vars_kwargs[category][heading]["all_files"][file_name] = { + "url": url, + "default": default, + "value": value, + } + except Exception: + pass + + def decode_b64(value: str): """Decodes the base64 representation back to utf-8 string.""" return base64.urlsafe_b64decode(value).decode("utf-8") From cd9cb910243728b2bfff8c9e05a82aa887411c06 Mon Sep 17 00:00:00 2001 From: Dagonite Date: Fri, 24 Dec 2021 09:55:33 +0000 Subject: [PATCH 10/10] Render config files --- .../templates/snippets/variables/render.html | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/autoreduce_frontend/templates/snippets/variables/render.html b/autoreduce_frontend/templates/snippets/variables/render.html index 39cd3d835..864b85c25 100644 --- a/autoreduce_frontend/templates/snippets/variables/render.html +++ b/autoreduce_frontend/templates/snippets/variables/render.html @@ -6,24 +6,32 @@
{% if variable.current|lower == "true" or variable.current|lower == "false" %} - + + {% elif "file" in name %} + {% else %} - + {% endif %}