From e43d5bab42875bce573b4765c8c7d04255e659c1 Mon Sep 17 00:00:00 2001 From: Alexander Goscinski Date: Tue, 24 Sep 2024 12:24:30 +0200 Subject: [PATCH] Check results are now collected before handling When multiple checks were handled only the output of the last check remained. This is because all checks were individually handled and each handling cleared the output. We now collect the check result including the exceptions and then send it for handling. We also now only print a summary of the results in the CheckRegistry as the output was too extensive. --- src/scwidgets/check/__init__.py | 4 +- src/scwidgets/check/_check.py | 12 +-- src/scwidgets/check/_widget_check_registry.py | 100 +++++++++++------- .../exercise/_widget_code_exercise.py | 41 ++++--- tests/test_check.py | 46 ++++---- tests/test_code.py | 24 +++-- 6 files changed, 128 insertions(+), 99 deletions(-) diff --git a/src/scwidgets/check/__init__.py b/src/scwidgets/check/__init__.py index a2dd8f7..11055f9 100644 --- a/src/scwidgets/check/__init__.py +++ b/src/scwidgets/check/__init__.py @@ -5,12 +5,12 @@ assert_shape, assert_type, ) -from ._check import AssertResult, Check, ChecksResult +from ._check import AssertResult, Check, CheckResult from ._widget_check_registry import CheckableWidget, CheckRegistry __all__ = [ "Check", - "ChecksResult", + "CheckResult", "AssertResult", "CheckRegistry", "CheckableWidget", diff --git a/src/scwidgets/check/_check.py b/src/scwidgets/check/_check.py index 2aa0fbd..1f3c8ab 100644 --- a/src/scwidgets/check/_check.py +++ b/src/scwidgets/check/_check.py @@ -181,7 +181,7 @@ def compute_outputs(self): def compute_and_set_references(self): self._outputs_references = self.compute_outputs() - def check_function(self) -> ChecksResult: + def check_function(self) -> CheckResult: """ Returns for each input (first depth list) the result message for each assert (second depth list). If a result message is empty, the assert was successful, @@ -199,7 +199,7 @@ def check_function(self) -> ChecksResult: f"[{len(self._inputs_parameters)} != {len(self._outputs_references)}]." ) - check_result = ChecksResult() + check_result = CheckResult() for i, input_parameters in enumerate(self._inputs_parameters): output = self._function_to_check(**input_parameters) if not (isinstance(output, tuple)): @@ -265,7 +265,7 @@ def check_function(self) -> ChecksResult: return check_result -class ChecksResult: +class CheckResult: def __init__(self): self._assert_results = [] self._assert_names = [] @@ -288,12 +288,6 @@ def append( self._inputs_parameters.append(input_parameters) self._suppress_assert_messages.append(suppress_assert_message) - def extend(self, check_results: ChecksResult): - self._assert_results.extend(check_results._assert_results) - self._assert_names.extend(check_results._assert_names) - self._inputs_parameters.extend(check_results._inputs_parameters) - self._suppress_assert_messages.extend(check_results._suppress_assert_messages) - @property def successful(self): return ( diff --git a/src/scwidgets/check/_widget_check_registry.py b/src/scwidgets/check/_widget_check_registry.py index ccdba22..bcc8e34 100644 --- a/src/scwidgets/check/_widget_check_registry.py +++ b/src/scwidgets/check/_widget_check_registry.py @@ -8,7 +8,7 @@ from ipywidgets import Button, HBox, Layout, Output, VBox, Widget from .._utils import Formatter -from ._check import Check, ChecksResult +from ._check import Check, CheckResult class CheckableWidget: @@ -38,7 +38,9 @@ def compute_output_to_check( """ raise NotImplementedError("compute_output_to_check has not been implemented") - def handle_checks_result(self, result: Union[ChecksResult, Exception]) -> None: + def handle_checks_result( + self, results: List[Union[CheckResult, Exception]] + ) -> None: """ Function that controls how results of the checks are handled. """ @@ -102,7 +104,7 @@ def compute_and_set_references(self): self._check_registry.compute_and_set_references(self) - def check(self) -> Union[ChecksResult, Exception]: + def check(self) -> List[Union[CheckResult, Exception]]: if self._check_registry is None: raise ValueError( "No check registry given on initialization, " "check cannot be used" @@ -220,7 +222,7 @@ def compute_and_set_references(self, widget: Widget): try: check.compute_and_set_references() except Exception as exception: - widget.handle_checks_result(exception) + widget.handle_checks_result([exception]) raise exception def compute_outputs(self, widget: CheckableWidget): @@ -228,36 +230,39 @@ def compute_outputs(self, widget: CheckableWidget): try: return check.compute_outputs() except Exception as exception: - widget.handle_checks_result(exception) + widget.handle_checks_result([exception]) raise exception def compute_and_set_all_references(self): for widget in self._checks.keys(): self.compute_and_set_references(widget) - def check_widget(self, widget: CheckableWidget) -> Union[ChecksResult, Exception]: + def check_widget( + self, widget: CheckableWidget + ) -> List[Union[CheckResult, Exception]]: + checks_result = [] try: - results = ChecksResult() for check in self._checks[widget]: result = check.check_function() - results.extend(result) - widget.handle_checks_result(result) - return results + checks_result.append(result) + widget.handle_checks_result(checks_result) + return checks_result except Exception as exception: - widget.handle_checks_result(exception) - return exception + checks_result.append(exception) + widget.handle_checks_result(checks_result) + return checks_result def check_all_widgets( self, - ) -> OrderedDict[CheckableWidget, Union[ChecksResult, Exception]]: - messages: OrderedDict[CheckableWidget, Union[ChecksResult, Exception]] = ( + ) -> OrderedDict[CheckableWidget, List[Union[CheckResult, Exception]]]: + messages: OrderedDict[CheckableWidget, List[Union[CheckResult, Exception]]] = ( OrderedDict() ) for widget in self._checks.keys(): try: messages[widget] = self.check_widget(widget) except Exception as exception: - messages[widget] = exception + messages[widget] = [exception] return messages def _on_click_set_all_references_button(self, change: dict): @@ -276,47 +281,60 @@ def _on_click_check_all_widgets_button(self, change: dict): widgets_results = self.check_all_widgets() for widget, widget_results in widgets_results.items(): with self._output: - if isinstance(widget_results, Exception): + if wrong_types := [ + result + for result in widget_results + if not ( + isinstance(result, Exception) + or isinstance(result, CheckResult) + ) + ]: + raise ValueError( + f"Not supported result type {type(wrong_types[0])}. " + "Only results of type `Exception` and `CheckResult` " + "are supported." + ) + elif [ + result + for result in widget_results + if isinstance(result, Exception) + ]: print( Formatter.color_error_message( Formatter.format_title_message( - f"Widget {self._names[widget]} " f"raised error:" + f"Widget {self._names[widget]} raised error." ) ) ) - raise widget_results - elif isinstance(widget_results, ChecksResult): - if widget_results.successful: - print( - Formatter.color_success_message( - Formatter.format_title_message( - f"Widget {self._names[widget]} all checks " - f"were successful" - ) + + elif not [ + result + for result in widget_results + if isinstance(result, CheckResult) and not result.successful + ]: + print( + Formatter.color_success_message( + Formatter.format_title_message( + f"Widget {self._names[widget]} all checks " + f"were successful." ) ) - print(widget_results.message()) - else: - print( - Formatter.color_error_message( - Formatter.format_title_message( - f"Widget {self._names[widget]} not all checks " - "were successful:" - ) + ) + else: + print( + Formatter.color_error_message( + Formatter.format_title_message( + f"Widget {self._names[widget]} not all checks " + "were successful." ) ) - print(widget_results.message()) - else: - raise ValueError( - f"Not supported result type {type(widget_results)}. " - "Only results of type `Exception` and `CheckResult` " - "are supported." ) except Exception as exception: with self._output: print( Formatter.color_error_message( "Error raised while checking widgets:" - ) + ), + exception, ) raise exception diff --git a/src/scwidgets/exercise/_widget_code_exercise.py b/src/scwidgets/exercise/_widget_code_exercise.py index 257c9e2..8ff4138 100644 --- a/src/scwidgets/exercise/_widget_code_exercise.py +++ b/src/scwidgets/exercise/_widget_code_exercise.py @@ -11,7 +11,7 @@ from widget_code_input.utils import CodeValidationError from .._utils import Formatter -from ..check import Check, CheckableWidget, CheckRegistry, ChecksResult +from ..check import Check, CheckableWidget, CheckRegistry, CheckResult from ..code._widget_code_input import CodeInput from ..code._widget_parameter_panel import ParameterPanel from ..cue import ( @@ -527,7 +527,7 @@ def _on_click_check_action(self) -> bool: raised_error = False with self._output: try: - self.check() + self._check() except Exception as e: raised_error = True if python_version() >= "3.11": @@ -571,29 +571,35 @@ def _on_click_load_action(self) -> bool: raise e return not (raised_error) - def check(self) -> Union[ChecksResult, Exception]: - self._output.clear_output(wait=True) + def _check(self) -> List[Union[CheckResult, Exception]]: return CheckableWidget.check(self) + def run_check(self) -> None: + if self._check_button is not None: + self._check_button.click() + else: + self._on_click_check_action() + def compute_output_to_check(self, *args, **kwargs) -> Check.FunOutParamsT: return self.run_code(*args, **kwargs) - def handle_checks_result(self, result: Union[ChecksResult, Exception]): + def handle_checks_result(self, results: List[Union[CheckResult, Exception]]): self._output.clear_output(wait=True) with self._output: - if isinstance(result, Exception): - raise result - elif isinstance(result, ChecksResult): - if result.successful: - print(Formatter.color_success_message("Check was successful")) - print(Formatter.color_success_message("--------------------")) - print(result.message()) + for result in results: + if isinstance(result, Exception): + raise result + elif isinstance(result, CheckResult): + if result.successful: + print(Formatter.color_success_message("Check was successful")) + print(Formatter.color_success_message("--------------------")) + print(result.message()) + else: + print(Formatter.color_error_message("Check failed")) + print(Formatter.color_error_message("------------")) + print(result.message()) else: - print(Formatter.color_error_message("Check failed")) - print(Formatter.color_error_message("------------")) - print(result.message()) - else: - print(result) + print(result) def handle_save_result(self, result: Union[str, Exception]): self._output.clear_output(wait=True) @@ -671,6 +677,7 @@ def run_update(self): parameter is changed """ if self._update_button is not None: + # to also invoke the reset cue action, we click the cued button self._update_button.click() else: # we might be in update_mode "release" or "continuous" where no button is diff --git a/tests/test_check.py b/tests/test_check.py index 96d0b56..2f70511 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -7,7 +7,7 @@ Check, CheckableWidget, CheckRegistry, - ChecksResult, + CheckResult, assert_numpy_allclose, assert_numpy_floating_sub_dtype, assert_shape, @@ -186,7 +186,7 @@ def test_compute_and_set_references(self, check): check.compute_and_set_references() # now checks should be successful again result = check.check_function() - assert isinstance(result, ChecksResult) + assert isinstance(result, CheckResult) assert result.successful @pytest.mark.parametrize( @@ -200,7 +200,7 @@ def test_compute_and_set_references(self, check): ) def test_failing_check_all_widgets(self, check): result = check.check_function() - assert isinstance(result, ChecksResult) + assert isinstance(result, CheckResult) assert not (result.successful) def test_invalid_asserts_arguments_count(self): @@ -247,7 +247,7 @@ def compute_output_to_check(self): pass def handle_checks_result(self, result): - self.results.append(result) + self.results.extend(result) checkable_widget = MockCheckableWidget(check_registry) checkable_widget.compute_output_to_check = compute_output_to_check @@ -285,18 +285,20 @@ def test_successful_check_all_widgets(self, checks): widgets_results = check_registry.check_all_widgets() nb_conducted_asserts = 0 - for result in widgets_results.values(): - assert isinstance(result, ChecksResult) - assert result.successful - nb_conducted_asserts += len(result.assert_results) + for results in widgets_results.values(): + for result in results: + assert isinstance(result, CheckResult) + assert result.successful + nb_conducted_asserts += len(result.assert_results) assert nb_conducted_asserts == checkable_widget.nb_conducted_asserts assert len(checkable_widget.results) == len(checks) nb_conducted_asserts = 0 - for result in checkable_widget.results: - assert isinstance(result, ChecksResult) - assert result.successful - nb_conducted_asserts += len(result.assert_results) + for results in widgets_results.values(): + for result in results: + assert isinstance(result, CheckResult) + assert result.successful + nb_conducted_asserts += len(result.assert_results) assert nb_conducted_asserts == checkable_widget.nb_conducted_asserts @pytest.mark.parametrize( @@ -323,16 +325,17 @@ def test_compute_and_set_all_references(self, checks): widgets_results = check_registry.check_all_widgets() nb_conducted_asserts = 0 - for result in widgets_results.values(): - assert isinstance(result, ChecksResult) - assert result.successful - nb_conducted_asserts += len(result.assert_results) + for results in widgets_results.values(): + for result in results: + assert isinstance(result, CheckResult) + assert result.successful + nb_conducted_asserts += len(result.assert_results) assert nb_conducted_asserts == checkable_widget.nb_conducted_asserts nb_conducted_asserts = 0 assert len(checkable_widget.results) == len(checks) for result in checkable_widget.results: - assert isinstance(result, ChecksResult) + assert isinstance(result, CheckResult) assert result.successful nb_conducted_asserts += len(result.assert_results) assert nb_conducted_asserts == checkable_widget.nb_conducted_asserts @@ -356,11 +359,12 @@ def test_failing_check_all_widgets(self, checks): ) widgets_results = check_registry.check_all_widgets() - for result in widgets_results.values(): - assert isinstance(result, ChecksResult) - assert not (result.successful) + for results in widgets_results.values(): + for result in results: + assert isinstance(result, CheckResult) + assert not (result.successful) assert len(checkable_widget.results) == len(checks) for result in checkable_widget.results: - assert isinstance(result, ChecksResult) + assert isinstance(result, CheckResult) assert not (result.successful) diff --git a/tests/test_code.py b/tests/test_code.py index 68a6139..8900721 100644 --- a/tests/test_code.py +++ b/tests/test_code.py @@ -6,7 +6,7 @@ from ipywidgets import fixed from widget_code_input.utils import CodeValidationError -from scwidgets.check import Check, CheckRegistry, ChecksResult +from scwidgets.check import Check, CheckRegistry, CheckResult from scwidgets.code import CodeInput from scwidgets.cue import CueObject from scwidgets.exercise import CodeExercise, ExerciseRegistry @@ -157,10 +157,13 @@ class TestCodeExercise: ], ) def test_successful_check_widget(self, code_ex): - result = code_ex.check() - assert isinstance(result, ChecksResult) - assert result.successful - assert len(result.assert_results) == code_ex.nb_conducted_asserts + results = code_ex.check() + nb_assert_results = 0 + for result in results: + assert isinstance(result, CheckResult) + assert result.successful + nb_assert_results += len(result.assert_results) + assert nb_assert_results == code_ex.nb_conducted_asserts @pytest.mark.parametrize( "code_ex", @@ -183,10 +186,13 @@ def test_successful_check_widget(self, code_ex): def test_compute_and_set_references(self, code_ex): code_ex.compute_and_set_references() - result = code_ex.check() - assert isinstance(result, ChecksResult) - assert result.successful - assert len(result.assert_results) == code_ex.nb_conducted_asserts + results = code_ex.check() + nb_assert_results = 0 + for result in results: + assert isinstance(result, CheckResult) + assert result.successful + nb_assert_results += len(result.assert_results) + assert nb_assert_results == code_ex.nb_conducted_asserts @pytest.mark.parametrize( "code_ex",