Repository: DiamondLightSource/httomo (language: python)

Issue:
YAML checker 
we need a tool that would help us checking the validity of parameters inside the YAML file. It is easy to make a mistake while typing. so we need to ensure the following checks:
- general inspection (a sanity check) of the YAML file, e.g., indentation
- module names and function names are valid
- parameters inside modules and their data types 
- the value of given parameters makes sense, e.g. within the range or the expected string, etc. 

regarding the data types check. The TomoPy checks could be different from httomolib. So far we need to satisfy the functions that are in this [database](https://github.com/DiamondLightSource/httomo/blob/main/httomo/methods_database/packages/tomopy.yaml) . Checking the types can be done through docstring inspection, if TomoPy functions are relatively consistent in naming. If not, we might need to add few bits into the database but as a last resort may be. 

I'm suggesting a totally separate script that the user can run before executing the httomo pipeline with a given YAML file. 


Discussion / hints:
Optionally the paths to the data and keys in the loader inside YAML file can be tested if the input data file is provided. If the data is not provided the paths in the loader are ignored. It is worth checking things like the validity of keys in the loader though, so we can encourage users to do so with something like `httomo check YAML_CONFIG IN_DATA`

Fix patch:
diff --git a/httomo/yaml_checker.py b/httomo/yaml_checker.py
new file mode 100644
index 000000000..7fcdfaaf9
--- /dev/null
+++ b/httomo/yaml_checker.py
@@ -0,0 +1,206 @@
+"""
+Module for checking the validity of yaml files.
+"""
+import os
+import yaml
+from typing import Any
+from httomo.utils import Colour
+from httomo.yaml_utils import open_yaml_config
+
+__all__ = [
+    "check_one_method_per_module",
+    "sanity_check",
+    "validate_yaml_config",
+]
+
+
+def sanity_check(yaml_file):
+    """
+    Check if the yaml file is properly indented, has valid mapping and tags.
+    """
+    _print_with_colour(
+        "Checking that the YAML_CONFIG is properly indented and has valid mappings and tags...",
+        colour=Colour.GREEN,
+    )
+    with open(yaml_file, "r") as file:
+        try:
+            yaml_data = open_yaml_config(yaml_file)
+            _print_with_colour(
+                "Sanity check of the YAML_CONFIG was successfully done...\n",
+                colour=Colour.GREEN,
+            )
+            return yaml_data
+        except yaml.parser.ParserError as e:
+            line = e.problem_mark.line
+            _print_with_colour(
+                f"Incorrect indentation in the YAML_CONFIG file at line {line}. "
+                "Please recheck the indentation of the file."
+            )
+        except yaml.scanner.ScannerError as e:
+            _print_with_colour(
+                f"Incorrect mapping in the YAML_CONFIG file at line {e.problem_mark.line + 1}."
+            )
+        except yaml.constructor.ConstructorError as e:
+            _print_with_colour(
+                f"Invalid tag in the YAML_CONFIG file at line {e.problem_mark.line + 1}."
+            )
+        except yaml.reader.ReaderError as e:
+            _print_with_colour(
+                f"Failed to parse YAML file at line {e.problem_mark.line + 1}: {e}"
+            )
+        except yaml.YAMLError as e:
+            if hasattr(e, "problem_mark"):
+                _print_with_colour(
+                    f"Error in the YAML_CONFIG file at line {e.problem_mark.line}. "
+                    "Please recheck the file."
+                )
+
+
+def check_one_method_per_module(yaml_file):
+    """
+    Check that we cannot have a yaml file with more than one method
+    being called from one module. For example, we cannot have:
+
+    - tomopy.prep.normalize:
+        normalize:
+          data_in: tomo
+          data_out: tomo
+          cutoff: null
+        minus_log:
+          data_in: tomo
+          data_out: tomo
+    """
+    _print_with_colour(
+        "Checking that YAML_CONFIG includes only one method from each module...\n"
+        "\nDoing a sanity check first...",
+        colour=Colour.GREEN,
+    )
+    yaml_data = sanity_check(yaml_file)
+
+    lvalues = [value for d in yaml_data for value in d.values()]
+    for i, d in enumerate(lvalues):
+        assert isinstance(d, dict)
+        if len(d) != 1:
+            _print_with_colour(
+                f"More than one method is being called from the"
+                f" module '{next(iter(yaml_data[i]))}'. "
+                "Please recheck the yaml file."
+            )
+            return False
+
+    _print_with_colour(
+        "'One method per module' check was also successfully done...\n",
+        colour=Colour.GREEN,
+    )
+    return yaml_data
+
+
+def _print_with_colour(end_str: Any, colour: Any = Colour.RED) -> None:
+    if isinstance(end_str, list):
+        output = "".join(
+            [f"{colour}{out}{Colour.END}" for out, colour in zip(end_str, colour)]
+        )
+        print(output)
+    else:
+        print(colour + end_str + Colour.END)
+
+
+def validate_yaml_config(yaml_file) -> bool:
+    """
+    Check that the modules, methods, and parameters in the `YAML_CONFIG` file
+    are valid, and adhere to the same structure as in each corresponding
+    module in `httomo.templates`.
+    """
+    yaml_data = check_one_method_per_module(yaml_file)
+
+    modules = [next(iter(d)) for d in yaml_data]
+    methods = [next(iter(d.values())) for d in yaml_data]
+    packages = [m.split(".")[0] for m in modules]
+
+    #: the first method is always a loader
+    #: so `testing_pipeline.yaml` should not pass.
+    _print_with_colour(
+        "Checking that the first method in the pipeline is a loader...",
+        colour=Colour.GREEN,
+    )
+    if modules[0] != "httomo.data.hdf.loaders":
+        _print_with_colour(
+            "The first method in the YAML_CONFIG file is not a loader from "
+            "'httomo.data.hdf.loaders'. Please recheck the yaml file."
+        )
+        return False
+    _print_with_colour("Loader check successful!!\n", colour=Colour.GREEN)
+
+    parent_dir = os.path.dirname(os.path.abspath("__file__"))
+    templates_dir = os.path.join(parent_dir, "templates")
+    assert os.path.exists(templates_dir)
+
+    _template_yaml_files = [
+        os.path.join(
+            templates_dir, packages[i], modules[i], next(iter(methods[i])) + ".yaml"
+        )
+        for i in range(len(modules))
+    ]
+
+    for i, f in enumerate(_template_yaml_files):
+        if not os.path.exists(f):
+            _print_with_colour(
+                f"'{modules[i] + '/' + next(iter(methods[i]))}' is not a valid"
+                " path to a method. Please recheck the yaml file."
+            )
+            return False
+
+    _template_yaml_data_list = [
+        next(iter(d.values()))
+        for f in _template_yaml_files
+        for d in open_yaml_config(f)
+    ]
+
+    for i, _ in enumerate(modules):
+        end_str_list = ["Checking '", next(iter(methods[i])), "' and its parameters..."]
+        colours = [Colour.GREEN, Colour.CYAN, Colour.GREEN]
+        _print_with_colour(end_str_list, colours)
+        d1 = methods[i]
+        d2 = _template_yaml_data_list[i]
+
+        for key in d1.keys():
+            for parameter in d1[key].keys():
+                assert isinstance(parameter, str)
+
+                # handle `data_in_multi` and `data_out_multi` as special cases
+                if parameter in ["data_in_multi", "data_out_multi"]:
+                    if not (
+                        isinstance(d1[key][parameter], list)
+                        and all(isinstance(x, str) for x in d1[key][parameter])
+                    ):
+                        _print_with_colour(
+                            f"Value assigned to parameter '{parameter}' in the '{modules[i]}' method"
+                            f" is not correct."
+                        )
+                        return False
+
+                    continue
+
+                if parameter not in d2[key].keys():
+                    _print_with_colour(
+                        f"Parameter '{parameter}' in the '{modules[i]}' method is not valid."
+                    )
+                    return False
+
+                # skip tuples for !Sweep and !SweepRange
+                if isinstance(d1[key][parameter], tuple) or None in (
+                    d1[key][parameter],
+                    d2[key][parameter],
+                ):
+                    continue
+
+                if not isinstance(d1[key][parameter], type(d2[key][parameter])):
+                    _print_with_colour(
+                        f"Value assigned to parameter '{parameter}' in the '{next(iter(methods[i]))}' method"
+                        f" is not correct. It should be of type {type(d2[key][parameter])}."
+                    )
+                    return False
+
+    end_str = "\nYAML validation successful!! Please feel free to use the `run` command to run the pipeline."
+    _print_with_colour(end_str, colour=Colour.BVIOLET)
+    return True
diff --git a/samples/pipeline_template_examples/multi_inputs/01_dezing_multi_inputs.yaml b/samples/pipeline_template_examples/multi_inputs/01_dezing_multi_inputs.yaml
index d9810d0a7..ab8d694db 100644
--- a/samples/pipeline_template_examples/multi_inputs/01_dezing_multi_inputs.yaml
+++ b/samples/pipeline_template_examples/multi_inputs/01_dezing_multi_inputs.yaml
@@ -4,7 +4,7 @@
       data_path: entry1/tomo_entry/data/data
       image_key_path: entry1/tomo_entry/instrument/detector/image_key
 - httomolib.misc.corr:
-    remove_outlier3d_cupy:
+    remove_outlier3d:
       data_in_multi: [tomo, flats, darks]
       data_out_multi: [tomo, flats, darks]
       kernel_size: 3


Test patch:
diff --git a/samples/pipeline_template_examples/testing/incorrect_method.yaml b/samples/pipeline_template_examples/testing/incorrect_method.yaml
new file mode 100644
index 000000000..983c3ff57
--- /dev/null
+++ b/samples/pipeline_template_examples/testing/incorrect_method.yaml
@@ -0,0 +1,22 @@
+- httomo.data.hdf.loaders:
+    standard_tomo:
+      name: tomo
+      data_path: entry1/tomo_entry/data/data
+      image_key_path: entry1/tomo_entry/instrument/detector/image_key
+      preview:
+        - 
+        - start: 30
+          stop: 60
+        - 
+      pad: 0
+- tomopy.misc.corr:
+    median_filters:
+      data: tomo
+      data_out: tomo
+      size: tomo
+      axis: 0
+- tomopy.prep.normalize:
+    normalize:
+      data_in: tomo
+      data_out: tomo
+      cutoff: null
diff --git a/samples/pipeline_template_examples/testing/more_than_one_method.yaml b/samples/pipeline_template_examples/testing/more_than_one_method.yaml
new file mode 100644
index 000000000..11392b9da
--- /dev/null
+++ b/samples/pipeline_template_examples/testing/more_than_one_method.yaml
@@ -0,0 +1,13 @@
+- httomo.data.hdf.loaders:
+    standard_tomo:
+      name: tomo
+      data_path: entry1/tomo_entry/data/data
+      image_key_path: entry1/tomo_entry/instrument/detector/image_key
+- tomopy.prep.normalize:
+    normalize:
+      data_in: tomo
+      data_out: tomo
+      cutoff: null
+    minus_log:
+      data_in: tomo
+      data_out: tomo
diff --git a/samples/pipeline_template_examples/testing/wrong_indentation_pipeline.yaml b/samples/pipeline_template_examples/testing/wrong_indentation_pipeline.yaml
new file mode 100644
index 000000000..a559bee6f
--- /dev/null
+++ b/samples/pipeline_template_examples/testing/wrong_indentation_pipeline.yaml
@@ -0,0 +1,7 @@
+- httomo.data.hdf.loaders:
+    standard_tomo:
+        name: tomo
+      data_path: /entry1/tomo_entry/data/data
+      image_key_path: /entry1/tomo_entry/instrument/detector/image_key
+      dimension: 1
+      pad: 0
diff --git a/tests/conftest.py b/tests/conftest.py
index 6e458684e..772c953ae 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -89,6 +89,16 @@ def standard_loader():
     return "samples/loader_configs/standard_tomo.yaml"
 
 
+@pytest.fixture
+def more_than_one_method():
+    return "samples/pipeline_template_examples/testing/more_than_one_method.yaml"
+
+
+@pytest.fixture
+def sample_pipelines():
+    return "samples/pipeline_template_examples/"
+
+
 @pytest.fixture
 def merge_yamls():
     def _merge_yamls(*yamls) -> None:
diff --git a/tests/test_yaml_checker.py b/tests/test_yaml_checker.py
new file mode 100644
index 000000000..d837a4026
--- /dev/null
+++ b/tests/test_yaml_checker.py
@@ -0,0 +1,44 @@
+"""
+Some unit tests for the yaml checker
+"""
+import pytest
+from httomo.yaml_checker import (
+    check_one_method_per_module,
+    sanity_check,
+    validate_yaml_config,
+)
+
+
+def test_sanity_check(sample_pipelines):
+    wrong_indentation_pipeline = (
+        sample_pipelines + "testing/wrong_indentation_pipeline.yaml"
+    )
+    assert not sanity_check(wrong_indentation_pipeline)
+
+
+def test_one_method_per_module(more_than_one_method):
+    assert not check_one_method_per_module(more_than_one_method)
+
+
+@pytest.mark.parametrize(
+    "yaml_file, expected",
+    [
+        ("testing/testing_pipeline.yaml", False),
+        ("testing/incorrect_method.yaml", False),
+        ("02_basic_cpu_pipeline_tomo_standard.yaml", True),
+        ("04_basic_gpu_pipeline_tomo_standard.yaml", True),
+        ("multi_inputs/01_dezing_multi_inputs.yaml", True),
+        ("parameter_sweeps/02_median_filter_kernel_sweep.yaml", True),
+    ],
+    ids=[
+        "no_loader_pipeline",
+        "incorrect_method",
+        "cpu_pipeline",
+        "gpu_pipeline",
+        "multi_input_pipeline",
+        "sweep_pipeline",
+    ],
+)
+def test_validate_yaml_config(sample_pipelines, yaml_file, expected):
+    yaml_file = sample_pipelines + yaml_file
+    assert validate_yaml_config(yaml_file) == expected