Update operations validation method to have access to the ImageStack (#2824)

Author: ashleymeigh2

docs/release_notes/next/dev-2795-ops-validation

@@ -0,0 +1 @@
+#2795: Allow operation validation method to access image stack

mantidimaging/core/operations/base_filter.py

@@ -88,8 +88,15 @@ def sv_params() -> dict[str, Any]:
         return {}
 
     @staticmethod
-    def validate_execute_kwargs(kwargs: dict[str, Any]) -> bool:
-        return True
+    def validate_execute_kwargs(kwargs: dict[str, Any], images: ImageStack) -> str | None:
+        """
+        Check operation parameters to highlight issues that need to be caught before calling filter_func()
+
+        The method also has access to the ImageStack so it can check dimensions and metadata if needed.
+
+        :return: None if no issues, a string message if the is a problem
+        """
+        return None
 
     @staticmethod
     def group_name() -> FilterGroup:

mantidimaging/core/operations/flat_fielding/flat_fielding.py

@@ -264,19 +264,19 @@ def execute_wrapper(  # type: ignore
                        use_dark=use_dark)
 
     @staticmethod
-    def validate_execute_kwargs(kwargs):
+    def validate_execute_kwargs(kwargs: dict[str, Any], images: ImageStack) -> str | None:
         # Validate something is in both path text inputs
         if 'selected_flat_fielding_widget' not in kwargs:
-            return False
+            return "Not all required parameters specified"
 
         if 'flat_before_widget' not in kwargs and 'dark_before_widget' not in kwargs or\
                 'flat_after_widget' not in kwargs and 'dark_after_widget' not in kwargs:
-            return False
+            return "Not all required parameters specified"
         assert isinstance(kwargs["flat_before_widget"], DatasetSelectorWidgetView)
         assert isinstance(kwargs["flat_after_widget"], DatasetSelectorWidgetView)
         assert isinstance(kwargs["dark_before_widget"], DatasetSelectorWidgetView)
         assert isinstance(kwargs["dark_after_widget"], DatasetSelectorWidgetView)
-        return True
+        return None
 
     @staticmethod
     def group_name() -> FilterGroup:

mantidimaging/core/operations/monitor_normalisation/monitor_normalisation.py

@@ -37,7 +37,7 @@ def filter_func(images: ImageStack, progress=None) -> ImageStack:
         if images.num_projections == 1:
             # we can't really compute the preview as the image stack copy
             # passed in doesn't have the logfile in it
-            raise RuntimeError("No logfile available for this stack.")
+            return images
 
         counts = images.counts()
         if counts is None:
@@ -63,5 +63,11 @@ def execute_wrapper(*args) -> partial:
         return partial(MonitorNormalisation.filter_func)
 
     @staticmethod
-    def validate_execute_kwargs(kwargs: dict[str, Any]) -> bool:
-        return True
+    def validate_execute_kwargs(kwargs: dict[str, Any], images: ImageStack) -> str | None:
+        counts = images.counts()
+        if counts is None:
+            return "Image stack has no counts. Ensure these have been loaded with a log file"
+
+        if counts.value.size != images.num_projections:
+            return "Number of entries in counts does not match number of images"
+        return None

mantidimaging/core/operations/monitor_normalisation/test/monitor_normalisation_test.py

@@ -20,22 +20,34 @@ def test_one_projection(self):
         images = generate_images((1, 1, 1))
         images._log_file = mock.Mock()
         images._log_file.counts = mock.Mock(return_value=Counts(np.sin(np.linspace(0, 1, images.num_projections))))
-        self.assertRaises(RuntimeError, MonitorNormalisation.filter_func, images)
+        result = MonitorNormalisation.filter_func(images)
+        self.assertIs(images, result)
 
     def test_no_counts(self):
         images = generate_images((2, 2, 2))
         images._log_file = mock.Mock()
         images._log_file.counts = mock.Mock(return_value=None)
+
+        self.assertIn("no counts", MonitorNormalisation.validate_execute_kwargs({}, images))
         self.assertRaises(RuntimeError, MonitorNormalisation.filter_func, images)
 
+    def test_wrong_number_of_counts(self):
+        images = generate_images()
+        images._log_file = mock.Mock()
+        images._log_file.counts = mock.Mock(return_value=Counts(np.sin(np.linspace(0, 1, images.num_projections + 1))))
+
+        self.assertIn("counts does not match number of images", MonitorNormalisation.validate_execute_kwargs({},
+                                                                                                             images))
+
     def test_execute(self):
         images = generate_images()
         images._log_file = mock.Mock()
         images._log_file.counts = mock.Mock(return_value=Counts(np.sin(np.linspace(0, 1, images.num_projections))))
 
         original = images.copy()
+        self.assertIsNone(MonitorNormalisation.validate_execute_kwargs({}, images))
         MonitorNormalisation.filter_func(images)
-        images._log_file.counts.assert_called_once()
+        images._log_file.counts.assert_called()
         self.assertEqual(original.data.shape, original.data.shape)
         assert_not_equals(original.data, images.data)
 
@@ -51,7 +63,7 @@ def test_execute2(self):
 
         original = images.copy()
         MonitorNormalisation.filter_func(images)
-        images._log_file.counts.assert_called_once()
+        images._log_file.counts.assert_called()
         npt.assert_equal(original.data, images.data)
 
     def test_register_gui(self):

mantidimaging/core/operations/overlap_correction/overlap_correction.py

@@ -5,7 +5,7 @@
 
 from functools import partial
 from pathlib import Path
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING
 from collections.abc import Callable
 from logging import getLogger
 
@@ -76,10 +76,6 @@ def register_gui(form: QFormLayout, on_change: Callable, view: BaseMainWindowVie
     def execute_wrapper(*args) -> partial:
         return partial(OverlapCorrection.filter_func)
 
-    @staticmethod
-    def validate_execute_kwargs(kwargs: dict[str, Any]) -> bool:
-        return True
-
     @staticmethod
     def get_shutters(data_dir: Path) -> list[ShutterInfo]:
         shutter_count_file = sorted(data_dir.glob("*ShutterCount.txt"))[0]

mantidimaging/core/operations/rescale/rescale.py

@@ -115,7 +115,3 @@ def execute_wrapper(  # type: ignore
             max_output = 2147483647.0
 
         return partial(RescaleFilter.filter_func, min_input=min_input, max_input=max_input, max_output=max_output)
-
-    @staticmethod
-    def validate_execute_kwargs(kwargs: dict[str, Any]) -> bool:
-        return True

mantidimaging/gui/windows/operations/model.py

@@ -120,13 +120,11 @@ def apply_to_stacks(self, stacks: list[ImageStack], progress=None):
         it to the function that actually processes the images.
         """
         for stack in stacks:
+            self.validate_kwargs(stack)
             self.apply_to_images(stack, progress=progress)
 
     def apply_to_images(self, images: ImageStack, progress=None, is_preview=False) -> None:
         input_kwarg_widgets = self.filter_widget_kwargs.copy()
-        # Validate required kwargs are supplied so pre-processing does not happen unnecessarily
-        if not self.selected_filter.validate_execute_kwargs(input_kwarg_widgets):
-            raise ValueError("Not all required parameters specified")
 
         # Run filter
         exec_func = self.selected_filter.execute_wrapper(**input_kwarg_widgets)
@@ -149,6 +147,14 @@ def apply_to_images(self, images: ImageStack, progress=None, is_preview=False) -
             *exec_func.args,
             **exec_func.keywords)
 
+    def validate_kwargs(self, images: ImageStack) -> None:
+        """
+        Validate required kwargs are supplied so pre-processing does not happen unnecessarily
+        """
+        input_kwarg_widgets = self.filter_widget_kwargs.copy()
+        if (message := self.selected_filter.validate_execute_kwargs(input_kwarg_widgets, images)) is not None:
+            raise ValueError(f"Issue with operation inputs: {message}")
+
     def do_apply_filter(self, stacks: list[ImageStack], post_filter: Callable[[Any], None]):
         """
         Applies the selected filter to the selected stack.

mantidimaging/gui/windows/operations/presenter.py

@@ -354,6 +354,9 @@ def _do_apply_filter_sync(self, apply_to):
         self.model.do_apply_filter_sync(apply_to, partial(self._post_filter, apply_to))
 
     def do_update_previews(self) -> None:
+        if not self.view.window_ready:
+            return
+
         if self.stack is None:
             self.view.clear_previews()
             return
@@ -383,8 +386,8 @@ def do_update_previews(self) -> None:
         # Take copies for display to prevent issues when shared memory is cleaned
         before_image = np.copy(subset.data.squeeze(squeeze_axis))
         try:
-            if self.model.filter_widget_kwargs:
-                self.model.apply_to_images(subset, is_preview=True)
+            self.model.validate_kwargs(self.stack)
+            self.model.apply_to_images(subset, is_preview=True)
         except Exception as e:
             msg = f"Error applying filter for preview: {e}"
             self.show_error(msg, traceback.format_exc())

mantidimaging/gui/windows/operations/test/model_test.py

@@ -46,7 +46,7 @@ def setup_mocks(self, execute_mock):
         orig_exec, orig_validate = f.execute_wrapper, f.validate_execute_kwargs
         self.model.setup_filter("", {})
         f.execute_wrapper = lambda: execute_mock
-        f.validate_execute_kwargs = lambda _: True
+        f.validate_execute_kwargs = lambda _, __: None
 
         return orig_exec, orig_validate
 
@@ -144,6 +144,7 @@ def test_apply_filter_to_images(self):
         selected_filter_mock.__name__ = mock.Mock()
         selected_filter_mock.__name__.return_value = "Test filter"
         selected_filter_mock.filter_name.return_value = "Test filter"
+        selected_filter_mock.validate_execute_kwargs.return_value = None
         progress_mock = mock.Mock()
 
         callback_mock = mock.Mock()
@@ -152,7 +153,6 @@ def test_apply_filter_to_images(self):
         self.model.selected_filter = selected_filter_mock
         self.model.apply_to_images(images, progress=progress_mock)
 
-        selected_filter_mock.validate_execute_kwargs.assert_called_once()
         callback_mock.assert_called_once_with(images, progress=progress_mock)
 
     def test_get_filter_module_name(self):