Repository: matchms/matchms

Issue:
Add default filters to yaml file
Any default settings of functions are not shown in the yaml file. This is bad for reproducibility, since changing any default settings in a filter function will currently result in a different pipeline running. 

Suggested change:
Automatically get the default filter settings from each filter and returning them in properties. (this will be used to create yaml files and printing the log file)

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 135242fa..09cb8c1c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,8 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [unreleased]
 
-## [0.22.0] - 2023-08-18
-
 ### Added
 
 - New `SpectrumProcessing` class to be the central hub for all filter functions [#455](https://github.com/matchms/matchms/pull/455). Also takes care that filters are executed in a useful order. This is also integrated into the `Pipeline` class.
@@ -562,10 +560,7 @@ Contains three plot types: `plot_spectrum()` or `spectrum.plot()`, `plot_spectra
 - This is the initial version of Spec2Vec from https://github.com/iomega/Spec2Vec
 - (later splitted into matchms + spec2vec)
 
-[Unreleased]: https://github.com/matchms/matchms/compare/0.22.0...HEAD
-[0.22.0]: https://github.com/matchms/matchms/compare/0.21.2...0.22.0
-[0.21.2]: https://github.com/matchms/matchms/compare/0.20.1...0.21.2
-[0.21.1]: https://github.com/matchms/matchms/compare/0.21.0...0.21.1
+[Unreleased]: https://github.com/matchms/matchms/compare/0.21.0...HEAD
 [0.21.0]: https://github.com/matchms/matchms/compare/0.20.0...0.21.0
 [0.20.0]: https://github.com/matchms/matchms/compare/0.19.0...0.20.0
 [0.19.0]: https://github.com/matchms/matchms/compare/0.18.0...0.19.0
diff --git a/matchms/Pipeline.py b/matchms/Pipeline.py
index baf579bb..32214b4e 100644
--- a/matchms/Pipeline.py
+++ b/matchms/Pipeline.py
@@ -214,24 +214,25 @@ class Pipeline:
         self.__workflow = workflow
         self.check_workflow()
 
-        self.write_to_logfile("--- Processing pipeline: ---")
         self._initialize_spectrum_processor_queries()
         if self.is_symmetric is False:
             self._initialize_spectrum_processor_references()
 
     def _initialize_spectrum_processor_queries(self):
         """Initialize spectrum processing workflow for the query spectra."""
+        self.write_to_logfile("--- Processing pipeline query spectra: ---")
         self.processing_queries = initialize_spectrum_processor(
             None,
             self.__workflow["query_filters"]
             )
-
         self.write_to_logfile(str(self.processing_queries))
         if self.processing_queries.processing_steps != self.__workflow["query_filters"]:
             logger.warning("The order of the filters has been changed compared to the Yaml file.")
 
     def _initialize_spectrum_processor_references(self):
         """Initialize spectrum processing workflow for the reference spectra."""
+        self.write_to_logfile("--- Processing pipeline reference spectra: ---")
+
         self.processing_references = initialize_spectrum_processor(
             None,
             self.__workflow["reference_filters"]
diff --git a/matchms/SpectrumProcessor.py b/matchms/SpectrumProcessor.py
index 51964296..090c3f4b 100644
--- a/matchms/SpectrumProcessor.py
+++ b/matchms/SpectrumProcessor.py
@@ -1,6 +1,7 @@
+import inspect
 from collections import defaultdict
 from functools import partial
-from typing import Optional, Tuple, Union
+from typing import Dict, Optional, Tuple, Union
 import numpy as np
 import pandas as pd
 from tqdm import tqdm
@@ -30,10 +31,10 @@ class SpectrumProcessor:
                 raise ValueError("Predefined pipeline parameter should be a string")
             if predefined_pipeline not in PREDEFINED_PIPELINES:
                 raise ValueError(f"Unknown processing pipeline '{predefined_pipeline}'. Available pipelines: {list(PREDEFINED_PIPELINES.keys())}")
-            for fname in PREDEFINED_PIPELINES[predefined_pipeline]:
-                self.add_matchms_filter(fname)
+            for filter_name in PREDEFINED_PIPELINES[predefined_pipeline]:
+                self.add_matchms_filter(filter_name)
 
-    def add_filter(self, filter_function: Union[Tuple[str], str]):
+    def add_filter(self, filter_function: Union[Tuple[str, Dict[str, any]], str]):
         """Add a filter to the processing pipeline. Takes both matchms filter names (and parameters)
         as well as custom-made functions.
         """
@@ -44,7 +45,7 @@ class SpectrumProcessor:
         else:
             self.add_custom_filter(filter_function[0], filter_function[1])
 
-    def add_matchms_filter(self, filter_spec: Union[Tuple[str], str]):
+    def add_matchms_filter(self, filter_spec: Union[Tuple[str, Dict[str, any]], str]):
         """
         Add a filter to the processing pipeline.
 
@@ -66,7 +67,7 @@ class SpectrumProcessor:
             filter_func.__name__ = FILTER_FUNCTIONS[filter_name].__name__
         else:
             raise TypeError("filter_spec should be a string or a tuple or list")
-
+        check_all_parameters_given(filter_func)
         self.filters.append(filter_func)
         # Sort filters according to their order in self.filter_order
         self.filters.sort(key=lambda f: self.filter_order.index(f.__name__))
@@ -93,12 +94,12 @@ class SpectrumProcessor:
             if filter_func.__name__ in self.filter_order:
                 filter_position = self.filter_order.index(filter_func.__name__)
         self.filter_order.insert(filter_position + 1, filter_function.__name__)
-        if filter_params is None:
-            self.filters.append(filter_function)
-        else:
-            filter_func = partial(filter_function, **filter_params)
-            filter_func.__name__ = filter_function.__name__
-            self.filters.append(filter_func)
+        if filter_params is not None:
+            partial_filter_func = partial(filter_function, **filter_params)
+            partial_filter_func.__name__ = filter_function.__name__
+            filter_function = partial_filter_func
+        check_all_parameters_given(filter_function)
+        self.filters.append(filter_function)
 
     def process_spectrum(self, spectrum,
                          processing_report: Optional["ProcessingReport"] = None):
@@ -171,11 +172,54 @@ class SpectrumProcessor:
 
     @property
     def processing_steps(self):
-        return [x.__name__ for x in self.filters]
+        filter_list = []
+        for filter_step in self.filters:
+            parameter_settings = get_parameter_settings(filter_step)
+            if parameter_settings is not None:
+                filter_list.append((filter_step.__name__, parameter_settings))
+            else:
+                filter_list.append(filter_step.__name__)
+        return filter_list
 
     def __str__(self):
-        summary_string = "SpectrumProcessor\nProcessing steps:\n - "
-        return summary_string + "\n - ".join(self.processing_steps)
+        summary_string = "SpectrumProcessor\nProcessing steps:"
+        for processing_step in self.processing_steps:
+            if isinstance(processing_step, str):
+                summary_string += "\n- " + processing_step
+            elif isinstance(processing_step, tuple):
+                filter_name = processing_step[0]
+                summary_string += "\n- - " + filter_name
+                filter_params = processing_step[1]
+                for filter_param in filter_params:
+                    summary_string += "\n  - " + str(filter_param)
+        return summary_string
+
+
+def check_all_parameters_given(func):
+    """Asserts that all added parameters for a function are given (except spectrum_in)"""
+    signature = inspect.signature(func)
+    parameters_without_value = []
+    for parameter, value in signature.parameters.items():
+        if value.default is inspect.Parameter.empty:
+            parameters_without_value.append(parameter)
+    assert len(parameters_without_value) == 1, \
+        f"More than one parameter of the function {func.__name__} is not specified, " \
+        f"the parameters not specified are {parameters_without_value}"
+
+
+def get_parameter_settings(func):
+    """Returns all parameters and parameter values for a function
+
+    This includes default parameter settings and, but also the settings stored in partial"""
+    signature = inspect.signature(func)
+    parameter_settings = {
+            parameter: value.default
+            for parameter, value in signature.parameters.items()
+            if value.default is not inspect.Parameter.empty
+        }
+    if parameter_settings == {}:
+        return None
+    return parameter_settings
 
 
 # List all filters in a functionally working order
@@ -246,7 +290,6 @@ DEFAULT_FILTERS = BASIC_FILTERS \
        "harmonize_undefined_inchi",
        "harmonize_undefined_smiles",
        "repair_inchi_inchikey_smiles",
-       "repair_parent_mass_match_smiles_wrapper",
        "normalize_intensities",
     ]
 FULLY_ANNOTATED_PROCESSING = DEFAULT_FILTERS \
diff --git a/matchms/__version__.py b/matchms/__version__.py
index 81edede8..e9734d46 100644
--- a/matchms/__version__.py
+++ b/matchms/__version__.py
@@ -1,1 +1,1 @@
-__version__ = '0.22.0'
+__version__ = '0.21.2'
diff --git a/pyproject.toml b/pyproject.toml
index 07a8d7fc..e7bccd6d 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "matchms"
-version = "0.22.0"
+version = "0.21.2"
 description = "Python library for large-scale comparisons and processing of tandem mass spectral data"
 authors = ["matchms developer team <florian.huber@hs-duesseldorf.de>"]
 


Test patch:
diff --git a/tests/filtering/test_spectrum_processor.py b/tests/filtering/test_spectrum_processor.py
index 0cad682c..a34253e4 100644
--- a/tests/filtering/test_spectrum_processor.py
+++ b/tests/filtering/test_spectrum_processor.py
@@ -24,35 +24,53 @@ def spectrums():
 
 def test_filter_sorting_and_output():
     processing = SpectrumProcessor("default")
-    expected_filters = [
-        'make_charge_int',
-        'add_compound_name',
-        'derive_adduct_from_name',
-        'derive_formula_from_name',
-        'clean_compound_name',
-        'interpret_pepmass',
-        'add_precursor_mz',
-        'derive_ionmode',
-        'correct_charge',
-        'require_precursor_mz',
-        'add_parent_mass',
-        'harmonize_undefined_inchikey',
-        'harmonize_undefined_inchi',
-        'harmonize_undefined_smiles',
-        'repair_inchi_inchikey_smiles',
-        'repair_parent_mass_match_smiles_wrapper',
-        'normalize_intensities'
-        ]
-    actual_filters = [x.__name__ for x in processing.filters]
-    assert actual_filters == expected_filters
-    # 2nd way to access the filter names via processing_steps attribute:
+    expected_filters = ['make_charge_int',
+                        'add_compound_name',
+                        ('derive_adduct_from_name', {'remove_adduct_from_name': True}),
+                        ('derive_formula_from_name', {'remove_formula_from_name': True}),
+                        'clean_compound_name',
+                        'interpret_pepmass',
+                        'add_precursor_mz',
+                        'derive_ionmode',
+                        'correct_charge',
+                        ('require_precursor_mz', {'minimum_accepted_mz': 10.0}),
+                        ('add_parent_mass',
+                         {'estimate_from_adduct': True, 'overwrite_existing_entry': False}),
+                        ('harmonize_undefined_inchikey', {'aliases': None, 'undefined': ''}),
+                        ('harmonize_undefined_inchi', {'aliases': None, 'undefined': ''}),
+                        ('harmonize_undefined_smiles', {'aliases': None, 'undefined': ''}),
+                        'repair_inchi_inchikey_smiles',
+                        'normalize_intensities']
     assert processing.processing_steps == expected_filters
 
 
+@pytest.mark.parametrize("filter_step, expected", [
+    [("add_parent_mass", {'estimate_from_adduct': False}),
+     ('add_parent_mass', {'estimate_from_adduct': False, 'overwrite_existing_entry': False})],
+    ["derive_adduct_from_name",
+     ('derive_adduct_from_name', {'remove_adduct_from_name': True})],
+    [("require_correct_ionmode", {"ion_mode_to_keep": "both"}),
+     ("require_correct_ionmode", {"ion_mode_to_keep": "both"})],
+])
+def test_overwrite_default_settings(filter_step, expected):
+    """Test if both default settings and set settings are returned in processing steps"""
+    processor = SpectrumProcessor(None)
+    processor.add_filter(filter_step)
+    expected_filters = [expected]
+    assert processor.processing_steps == expected_filters
+
+
+def test_incomplete_parameters():
+    """Test if an error is raised when running an incomplete command"""
+    with pytest.raises(AssertionError):
+        processor = SpectrumProcessor(None)
+        processor.add_filter("require_correct_ionmode")
+
+
 def test_string_output():
     processing = SpectrumProcessor("minimal")
-    expected_str = "SpectrumProcessor\nProcessing steps:\n - make_charge_int\n - interpret_pepmass"\
-        "\n - derive_ionmode\n - correct_charge"
+    expected_str = "SpectrumProcessor\nProcessing steps:\n- make_charge_int\n- interpret_pepmass" \
+                   "\n- derive_ionmode\n- correct_charge"
     assert str(processing) == expected_str
 
 
@@ -65,7 +83,7 @@ def test_add_matchms_filter(metadata, expected):
     spectrum_in = SpectrumBuilder().with_metadata(metadata).build()
     processor = SpectrumProcessor("minimal")
     processor.add_matchms_filter(("require_correct_ionmode",
-                                 {"ion_mode_to_keep": "both"}))
+                                  {"ion_mode_to_keep": "both"}))
     spectrum = processor.process_spectrum(spectrum_in)
     if expected is None:
         assert spectrum is None
@@ -175,7 +193,7 @@ def test_add_filter_with_custom(spectrums):
 def test_add_filter_with_matchms_filter(spectrums):
     processor = SpectrumProcessor("minimal")
     processor.add_filter(("require_correct_ionmode",
-                         {"ion_mode_to_keep": "both"}))
+                          {"ion_mode_to_keep": "both"}))
     filters = processor.filters
     assert filters[-1].__name__ == "require_correct_ionmode"
     spectrums, _ = processor.process_spectrums(spectrums, create_report=True)
@@ -187,6 +205,7 @@ def test_all_filters_is_complete():
 
     This is important, since performing tests in the wrong order can make some filters useless.
     """
+
     def get_functions_from_file(file_path):
         """Gets all python functions in a file"""
         with open(file_path, 'r', encoding="utf-8") as file: