From d4268dc095d2c4649529342a6ec2561131ed901a Mon Sep 17 00:00:00 2001 From: Maria Patrou Date: Mon, 15 May 2023 12:20:48 -0400 Subject: [PATCH 1/2] generate button de/activation based on required fields, valueerror exceptions added, related tests updated --- src/shiver/models/generate.py | 2 +- src/shiver/models/histogram.py | 2 +- src/shiver/views/generate.py | 36 ++++++++++++++++-------- tests/views/test_generate.py | 50 +++++++++++++++++++++------------- 4 files changed, 57 insertions(+), 33 deletions(-) diff --git a/src/shiver/models/generate.py b/src/shiver/models/generate.py index c8f35521..11faef60 100644 --- a/src/shiver/models/generate.py +++ b/src/shiver/models/generate.py @@ -161,7 +161,7 @@ def generate_mde(self, config_dict: dict): alg.setProperty("UBParameters", ub_parameters) alg.setProperty("OutputWorkspace", output_workspace) alg.executeAsync() - except RuntimeError as err: + except (RuntimeError, ValueError) as err: # NOTE: this error is usually related to incorrect input that triggers # error during alg start-up, execution error will be captured # by the obs handlers. diff --git a/src/shiver/models/histogram.py b/src/shiver/models/histogram.py index a1de10f5..17e89100 100644 --- a/src/shiver/models/histogram.py +++ b/src/shiver/models/histogram.py @@ -330,7 +330,7 @@ def do_make_slice(self, config: dict): alg.setProperty("Smoothing", config.get("Smoothing", "")) alg.setProperty("OutputWorkspace", config.get("OutputWorkspace")) alg.executeAsync() - except RuntimeError as err: + except (RuntimeError, ValueError) as err: logger.error(str(err)) if self.error_callback: self.error_callback(str(err)) diff --git a/src/shiver/views/generate.py b/src/shiver/views/generate.py index 52280202..1cbf8ed8 100644 --- a/src/shiver/views/generate.py +++ b/src/shiver/views/generate.py @@ -88,10 +88,18 @@ def __init__(self, parent=None): self.inhibit_update = False - # check the state of the required fields - # pass the save_btn in mde_type widget to allow for button activations/deactivations + # check the state of the required fields for each button + # to allow for button activations/deactivations of save_btn and generate_btn # based on the fields states - self.field_errors = [] + self.field_errors = {self.buttons.save_btn: [], self.buttons.generate_btn: []} + # mandatory fields for the two available buttons + self.field_btns = { + self.mde_type_widget.output_dir: [self.buttons.save_btn], + self.mde_type_widget.mde_name: [self.buttons.save_btn], + self.raw_data_widget.files: [self.buttons.save_btn, self.buttons.generate_btn], + self.reduction_parameters.ei_input: [self.buttons.save_btn], + self.reduction_parameters.t0_input: [self.buttons.save_btn], + } self.mde_type_widget.check_output_dir() self.mde_type_widget.check_mde_name() self.raw_data_widget.check_file_input() @@ -203,17 +211,21 @@ def _show_error_message(self, msg): error.exec_() def set_field_invalid_state(self, item): - """include the item in the field_error list and disable the corresponding button""" - if item not in self.field_errors: - self.field_errors.append(item) - self.buttons.save_btn.setEnabled(False) + """include the item in the field_error list and disable the corresponding button/s""" + buttons = self.field_btns[item] + for btn in buttons: + if item not in self.field_errors[btn]: + self.field_errors[btn].append(item) + btn.setEnabled(False) def set_field_valid_state(self, item): - """remove the item from the field_error list and enable the corresponding button""" - if item in self.field_errors: - self.field_errors.remove(item) - if len(self.field_errors) == 0: - self.buttons.save_btn.setEnabled(True) + """remove the item from the field_error list and enable the corresponding button/s""" + buttons = self.field_btns[item] + for btn in buttons: + if item in self.field_errors[btn]: + self.field_errors[btn].remove(item) + if len(self.field_errors[btn]) == 0: + btn.setEnabled(True) def get_save_configuration_filepath( self, diff --git a/tests/views/test_generate.py b/tests/views/test_generate.py index c0ba1027..89ea9e98 100644 --- a/tests/views/test_generate.py +++ b/tests/views/test_generate.py @@ -40,7 +40,8 @@ def error_callback(msg): mde_type_widget.connect_error_callback(error_callback) # all three required fields are empty - assert len(generate.field_errors) == 3 + assert len(generate.field_errors[generate.buttons.save_btn]) == 3 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 1 # set files directory = os.path.realpath(os.path.join(os.path.dirname(os.path.abspath(__file__)), "../data/raw")) @@ -69,18 +70,21 @@ def error_callback(msg): assert mde_type_widget.mde_name.text() == "test" assert mde_type_widget.output_dir.text() == "/tmp/test" assert mde_type_widget.mde_type_background_integrated.isChecked() is True - assert len(generate.field_errors) == 0 + assert len(generate.field_errors[generate.buttons.save_btn]) == 0 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 0 # ref_dict["mde_type"] = "Data" mde_type_widget.populate_from_dict(ref_dict) assert mde_type_widget.mde_type_data.isChecked() is True - assert len(generate.field_errors) == 0 + assert len(generate.field_errors[generate.buttons.save_btn]) == 0 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 0 # ref_dict["mde_type"] = "Background (minimized by angle and energy)" mde_type_widget.populate_from_dict(ref_dict) assert mde_type_widget.mde_type_background_minimized.isChecked() is True - assert len(generate.field_errors) == 0 + assert len(generate.field_errors[generate.buttons.save_btn]) == 0 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 0 # check error_1: invalid mde name mde_type_widget.re_init_widget() @@ -88,14 +92,15 @@ def error_callback(msg): mde_type_widget.populate_from_dict(ref_dict) assert not mde_type_widget.as_dict() assert errors_list[-1] == "Invalid MDE name found in history." - assert len(generate.field_errors) == 1 + assert len(generate.field_errors[generate.buttons.save_btn]) == 1 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 0 # check error_2: empty output dir mde_type_widget.re_init_widget() mde_type_widget.mde_name.setText("test") mde_type_widget.output_dir.setText(" ") assert not mde_type_widget.as_dict() - assert len(generate.field_errors) == 1 + assert len(generate.field_errors[generate.buttons.save_btn]) == 1 # check error_3: invalid output dir mde_type_widget.re_init_widget() @@ -104,13 +109,13 @@ def error_callback(msg): mde_type_widget.populate_from_dict(ref_dict) assert not mde_type_widget.as_dict() assert errors_list[-1] == "Invalid output directory found in history." - assert len(generate.field_errors) == 2 + assert len(generate.field_errors[generate.buttons.save_btn]) == 2 # check error_4: invalid dict used to populate UI mde_type_widget.re_init_widget() mde_type_widget.populate_from_dict({"mde_name": "test?"}) assert errors_list[-1] == "Invalid MDE name found in history." - assert len(generate.field_errors) == 2 + assert len(generate.field_errors[generate.buttons.save_btn]) == 2 # mde_type_widget.re_init_widget() @@ -142,31 +147,34 @@ def test_generate_widget_colors_invalid(qtbot): color_search = re.compile("border-color: (.*);") # all three required fields are empty - assert len(generate.field_errors) == 3 - + assert len(generate.field_errors[generate.buttons.save_btn]) == 3 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 1 # check mde_name border qtbot.keyClicks(mde_type_widget.mde_name, "") css_style_mde_name = mde_type_widget.mde_name.styleSheet() color = color_search.search(css_style_mde_name).group(1) assert color == "red" - assert len(generate.field_errors) == 3 + assert len(generate.field_errors[generate.buttons.save_btn]) == 3 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 1 qtbot.keyClicks(mde_type_widget.output_dir, "/tmp/test?") # check output_dir border css_style_output_dir = mde_type_widget.output_dir.styleSheet() color = color_search.search(css_style_output_dir).group(1) assert color == "red" - assert len(generate.field_errors) == 3 + assert len(generate.field_errors[generate.buttons.save_btn]) == 3 # check files border css_style_files = raw_data_widget.files.styleSheet() color = color_search.search(css_style_files).group(1) assert color == "red" - assert len(generate.field_errors) == 3 + assert len(generate.field_errors[generate.buttons.save_btn]) == 3 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 1 - # assert button is deactivated + # assert buttons are deactivated assert generate.buttons.save_btn.isEnabled() is False + assert generate.buttons.generate_btn.isEnabled() is False def test_generate_widget_colors_valid(qtbot): @@ -177,19 +185,20 @@ def test_generate_widget_colors_valid(qtbot): qtbot.addWidget(generate) generate.show() - assert len(generate.field_errors) == 3 + assert len(generate.field_errors[generate.buttons.save_btn]) == 3 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 1 # set mde_name qtbot.keyClicks(mde_type_widget.mde_name, "mde_test_2") css_style_mde_name = mde_type_widget.mde_name.styleSheet() assert css_style_mde_name == "" - assert len(generate.field_errors) == 2 + assert len(generate.field_errors[generate.buttons.save_btn]) == 2 # set output_dir qtbot.keyClicks(mde_type_widget.output_dir, "/tmp/") css_style_output_dir = mde_type_widget.output_dir.styleSheet() assert css_style_output_dir == "" - assert len(generate.field_errors) == 1 + assert len(generate.field_errors[generate.buttons.save_btn]) == 1 # set files assert raw_data_widget.files.count() == 0 @@ -203,7 +212,10 @@ def test_generate_widget_colors_valid(qtbot): qtbot.wait(100) css_style_files = raw_data_widget.files.styleSheet() assert css_style_files == "" - assert len(generate.field_errors) == 0 - # assert button is activated + assert len(generate.field_errors[generate.buttons.save_btn]) == 0 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 0 + + # assert buttons are activated assert generate.buttons.save_btn.isEnabled() is True + assert generate.buttons.generate_btn.isEnabled() is True qtbot.mouseClick(generate.buttons.save_btn, QtCore.Qt.LeftButton) From 9f2d1c0c8943ba09c5b74e3ec667affd8a43714c Mon Sep 17 00:00:00 2001 From: Maria Patrou Date: Thu, 18 May 2023 11:26:37 -0400 Subject: [PATCH 2/2] generate button invalid files crash, required fields added fot btn de/activation, related tests updated --- src/shiver/models/convert_dgs_to_single_mde.py | 9 ++++++++- src/shiver/models/generate.py | 11 +++++++---- src/shiver/views/generate.py | 8 ++++---- tests/views/test_generate.py | 12 ++++++------ 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/shiver/models/convert_dgs_to_single_mde.py b/src/shiver/models/convert_dgs_to_single_mde.py index 3c35362e..b9569f82 100644 --- a/src/shiver/models/convert_dgs_to_single_mde.py +++ b/src/shiver/models/convert_dgs_to_single_mde.py @@ -264,7 +264,14 @@ def PyExec(self): # pylint: disable=too-many-branches run_obj = data.getRun() # check if monitor is necessary and get Ei,T0 if inst_name in ["HYSPEC", "CNCS"]: - Ei = Ei_supplied if Ei_supplied else run_obj["EnergyRequest"].getStatistics().mean + Ei = None + if Ei_supplied: + Ei = Ei_supplied + elif "EnergyRequest" in run_obj: + Ei = run_obj["EnergyRequest"].getStatistics().mean + else: + self.log().error("EnergyRequest is not defined") + raise ValueError("EnergyRequest is not defined") T0 = T0_supplied if (T0_supplied is not None) else GetEi(data).Tzero else: if (Ei_supplied is not None) and (T0_supplied is not None): diff --git a/src/shiver/models/generate.py b/src/shiver/models/generate.py index 11faef60..4495c7ec 100644 --- a/src/shiver/models/generate.py +++ b/src/shiver/models/generate.py @@ -47,7 +47,7 @@ def generate_mde(self, config_dict: dict): # disable the Generate button to prevent multiple clicks if self.generate_mde_finish_callback: - self.generate_mde_finish_callback() + self.generate_mde_finish_callback(False) # remove output workspace if it exists in memory output_workspace = config_dict.get("mde_name", "") @@ -190,8 +190,11 @@ def finish_generate_mde(self, obs, error=False, msg=""): self.workspace_name = None self.output_dir = None self.config_dict = None + # enable button if self.generate_mde_finish_callback: - self.generate_mde_finish_callback() + self.generate_mde_finish_callback(True) + if self.error_callback: + self.error_callback(msg=err_msg) else: logger.information("GenerateDGSMDE finished") # attach config_dict to the workspace @@ -262,8 +265,8 @@ def finish_save_md(self, obs, error=False, msg=""): self.output_dir = None self.algorithm_observer.remove(obs) - - self.generate_mde_finish_callback() + # enable button + self.generate_mde_finish_callback(True) class GenerateMDEObserver(AlgorithmObserver): diff --git a/src/shiver/views/generate.py b/src/shiver/views/generate.py index 1cbf8ed8..e708fe90 100644 --- a/src/shiver/views/generate.py +++ b/src/shiver/views/generate.py @@ -94,8 +94,8 @@ def __init__(self, parent=None): self.field_errors = {self.buttons.save_btn: [], self.buttons.generate_btn: []} # mandatory fields for the two available buttons self.field_btns = { - self.mde_type_widget.output_dir: [self.buttons.save_btn], - self.mde_type_widget.mde_name: [self.buttons.save_btn], + self.mde_type_widget.output_dir: [self.buttons.save_btn, self.buttons.generate_btn], + self.mde_type_widget.mde_name: [self.buttons.save_btn, self.buttons.generate_btn], self.raw_data_widget.files: [self.buttons.save_btn, self.buttons.generate_btn], self.reduction_parameters.ei_input: [self.buttons.save_btn], self.reduction_parameters.t0_input: [self.buttons.save_btn], @@ -104,9 +104,9 @@ def __init__(self, parent=None): self.mde_type_widget.check_mde_name() self.raw_data_widget.check_file_input() - def generate_mde_finish_callback(self): + def generate_mde_finish_callback(self, activate): """Toggle the generate button disabled state.""" - if self.isEnabled(): + if not activate: self.setDisabled(True) else: self.setEnabled(True) diff --git a/tests/views/test_generate.py b/tests/views/test_generate.py index 89ea9e98..4871c961 100644 --- a/tests/views/test_generate.py +++ b/tests/views/test_generate.py @@ -41,7 +41,7 @@ def error_callback(msg): # all three required fields are empty assert len(generate.field_errors[generate.buttons.save_btn]) == 3 - assert len(generate.field_errors[generate.buttons.generate_btn]) == 1 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 3 # set files directory = os.path.realpath(os.path.join(os.path.dirname(os.path.abspath(__file__)), "../data/raw")) @@ -93,7 +93,7 @@ def error_callback(msg): assert not mde_type_widget.as_dict() assert errors_list[-1] == "Invalid MDE name found in history." assert len(generate.field_errors[generate.buttons.save_btn]) == 1 - assert len(generate.field_errors[generate.buttons.generate_btn]) == 0 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 1 # check error_2: empty output dir mde_type_widget.re_init_widget() @@ -148,7 +148,7 @@ def test_generate_widget_colors_invalid(qtbot): # all three required fields are empty assert len(generate.field_errors[generate.buttons.save_btn]) == 3 - assert len(generate.field_errors[generate.buttons.generate_btn]) == 1 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 3 # check mde_name border qtbot.keyClicks(mde_type_widget.mde_name, "") @@ -156,7 +156,7 @@ def test_generate_widget_colors_invalid(qtbot): color = color_search.search(css_style_mde_name).group(1) assert color == "red" assert len(generate.field_errors[generate.buttons.save_btn]) == 3 - assert len(generate.field_errors[generate.buttons.generate_btn]) == 1 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 3 qtbot.keyClicks(mde_type_widget.output_dir, "/tmp/test?") # check output_dir border @@ -170,7 +170,7 @@ def test_generate_widget_colors_invalid(qtbot): color = color_search.search(css_style_files).group(1) assert color == "red" assert len(generate.field_errors[generate.buttons.save_btn]) == 3 - assert len(generate.field_errors[generate.buttons.generate_btn]) == 1 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 3 # assert buttons are deactivated assert generate.buttons.save_btn.isEnabled() is False @@ -186,7 +186,7 @@ def test_generate_widget_colors_valid(qtbot): generate.show() assert len(generate.field_errors[generate.buttons.save_btn]) == 3 - assert len(generate.field_errors[generate.buttons.generate_btn]) == 1 + assert len(generate.field_errors[generate.buttons.generate_btn]) == 3 # set mde_name qtbot.keyClicks(mde_type_widget.mde_name, "mde_test_2")