Repository: matchms/matchms
Pull request #498 (2023-08-21 14:16:30)

Spectrum processor sorting
The sorting algorithm was a bit counter intuitive. 
The custom_filters would be added just after the first of the already added filters.

I have now added the option to specify a position, which will just add it to the end if it is not given. 


Patch:
diff --git a/matchms/SpectrumProcessor.py b/matchms/SpectrumProcessor.py
index 090c3f4b..0ced8e24 100644
--- a/matchms/SpectrumProcessor.py
+++ b/matchms/SpectrumProcessor.py
@@ -72,7 +72,7 @@ class SpectrumProcessor:
         # Sort filters according to their order in self.filter_order
         self.filters.sort(key=lambda f: self.filter_order.index(f.__name__))
 
-    def add_custom_filter(self, filter_function, filter_params=None):
+    def add_custom_filter(self, filter_function, filter_params=None, filter_position: Optional[int] = None):
         """
         Add a custom filter function to the processing pipeline.
 
@@ -86,20 +86,31 @@ class SpectrumProcessor:
             processing pipeline.
         filter_params: dict
             If needed, add dictionary with all filter parameters. Default is set to None.
+        filter_position:
+            The position this filter should be inserted in the filter order.
+            If None, it will be appended at the end of the current list of filters.
         """
         if not callable(filter_function):
             raise TypeError("Expected callable filter function.")
-        filter_position = 0
-        for filter_func in self.filters[::-1]:
-            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_position is None:
+            self.filter_order.append(filter_function.__name__)
+        elif not isinstance(filter_position, int):
+            raise TypeError("Expected filter_position to be an integer.")
+        else:
+            if filter_position >= len(self.filters):
+                self.filter_order.append(filter_function.__name__)
+            else:
+                current_filter_at_position = self.filters[filter_position].__name__
+                order_index = self.filter_order.index(current_filter_at_position)
+                self.filter_order.insert(order_index, filter_function.__name__)
+
         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)
+        self.filters.sort(key=lambda f: self.filter_order.index(f.__name__))
 
     def process_spectrum(self, spectrum,
                          processing_report: Optional["ProcessingReport"] = None):


Test patch:
diff --git a/tests/filtering/test_spectrum_processor.py b/tests/filtering/test_spectrum_processor.py
index a34253e4..ee2abf0c 100644
--- a/tests/filtering/test_spectrum_processor.py
+++ b/tests/filtering/test_spectrum_processor.py
@@ -177,6 +177,28 @@ def test_adding_custom_filter_with_parameters(spectrums):
     assert spectrums[0].get("inchikey") == "NONSENSENONSENSE", "Custom filter not executed properly"
 
 
+@pytest.mark.parametrize("filter_position, expected", [
+    [0, 0],
+    [1, 1],
+    [2, 2],
+    [3, 3],
+    [None, 4],
+    [5, 4],
+    [6, 4]
+])
+def test_add_custom_filter_in_position(filter_position, expected):
+    def nonsense_inchikey_multiple(s, number):
+        s.set("inchikey", number * "NONSENSE")
+        return s
+
+    processor = SpectrumProcessor("minimal")
+    processor.add_custom_filter(nonsense_inchikey_multiple, {"number": 2},
+                                filter_position=filter_position)
+    filters = processor.filters
+
+    assert filters[expected].__name__ == "nonsense_inchikey_multiple"
+
+
 def test_add_filter_with_custom(spectrums):
     def nonsense_inchikey_multiple(s, number):
         s.set("inchikey", number * "NONSENSE")
@@ -185,6 +207,7 @@ def test_add_filter_with_custom(spectrums):
     processor = SpectrumProcessor("minimal")
     processor.add_filter((nonsense_inchikey_multiple, {"number": 2}))
     filters = processor.filters
+
     assert filters[-1].__name__ == "nonsense_inchikey_multiple"
     spectrums, _ = processor.process_spectrums(spectrums, create_report=True)
     assert spectrums[0].get("inchikey") == "NONSENSENONSENSE", "Custom filter not executed properly"