Repository: matchms/matchms
Pull request #562 (2023-11-14 16:20:42)

Create new predefined pipelines, with subgroups and update tests.
- [x] Created new subgroups
- [x] Update test that used old predefined pipelines
- [x] Add tests that all the defined pipelines in predefined_pipelines can actually run (basically that there are no typos). 
- [x] Split file into filter_order and predefined_pipelines



Patch:
diff --git a/matchms/Pipeline.py b/matchms/Pipeline.py
index 00673164..f91decd8 100644
--- a/matchms/Pipeline.py
+++ b/matchms/Pipeline.py
@@ -5,7 +5,7 @@ from datetime import datetime
 from typing import Any, Dict, Iterable, List, Optional, Tuple, Union
 import matchms.similarity as mssimilarity
 from matchms import calculate_scores
-from matchms.filtering.filter_order_and_default_pipelines import ALL_FILTERS
+from matchms.filtering.filter_order import ALL_FILTERS
 from matchms.filtering.SpectrumProcessor import SpectrumProcessor
 from matchms.importing.load_spectra import load_list_of_spectrum_files
 from matchms.logging_functions import (add_logging_to_file,
diff --git a/matchms/filtering/SpectrumProcessor.py b/matchms/filtering/SpectrumProcessor.py
index 55c43961..787bb18b 100644
--- a/matchms/filtering/SpectrumProcessor.py
+++ b/matchms/filtering/SpectrumProcessor.py
@@ -9,8 +9,8 @@ import pandas as pd
 from tqdm import tqdm
 from matchms import Spectrum
 from matchms.exporting import save_spectra
-from matchms.filtering.filter_order_and_default_pipelines import (
-    ALL_FILTERS, FILTER_FUNCTION_NAMES, PREDEFINED_PIPELINES)
+from matchms.filtering.default_pipelines import PREDEFINED_PIPELINES
+from matchms.filtering.filter_order import ALL_FILTERS, FILTER_FUNCTION_NAMES
 from matchms.yaml_file_functions import ordered_dump
 
 
diff --git a/matchms/filtering/default_pipelines.py b/matchms/filtering/default_pipelines.py
new file mode 100644
index 00000000..f5f0eee8
--- /dev/null
+++ b/matchms/filtering/default_pipelines.py
@@ -0,0 +1,65 @@
+"""Stores sets of filters for easy pipeline creation."""
+
+HARMONIZE_METADATA_FIELD_NAMES = \
+    ["make_charge_int",
+     "add_compound_name",
+     "interpret_pepmass",
+     "add_precursor_mz",
+     "add_retention_time",
+     "add_retention_index",
+     ]
+DERIVE_METADATA_IN_WRONG_FIELD = \
+    ["derive_adduct_from_name",
+     "derive_formula_from_name",
+     "clean_compound_name",
+     "derive_ionmode",
+     "repair_inchi_inchikey_smiles"
+     ]
+HARMONIZE_METADATA_ENTRIES = \
+    ["harmonize_undefined_inchikey",
+     "harmonize_undefined_inchi",
+     "harmonize_undefined_smiles",
+     "clean_adduct",
+     ]
+DERIVE_MISSING_METADATA = ["correct_charge", "add_parent_mass", ]
+REQUIRE_COMPLETE_METADATA = ["require_precursor_mz",
+                             ("require_correct_ionmode", {"ion_mode_to_keep": "both"}), ]
+REPAIR_ANNOTATION = [
+    "derive_inchi_from_smiles",
+    "derive_smiles_from_inchi",
+    "derive_inchikey_from_inchi",
+    ("repair_smiles_of_salts", {'mass_tolerance': 0.1}),
+    ("repair_precursor_is_parent_mass", {'mass_tolerance': 0.1}),
+    ("repair_parent_mass_is_mol_wt", {'mass_tolerance': 0.1}),
+    ("repair_adduct_based_on_smiles", {'mass_tolerance': 0.1}),
+    "repair_not_matching_annotation",
+    ("derive_annotation_from_compound_name", {"mass_tolerance": 0.1}),
+]
+REQUIRE_COMPLETE_ANNOTATION = [("require_parent_mass_match_smiles", {'mass_tolerance': 0.1}),
+                               "require_valid_annotation", ]
+CLEAN_PEAKS = [("select_by_mz", {"mz_from": 0, "mz_to": 1000}),
+               ("select_by_relative_intensity", {"intensity_from": 0.001}),
+               ("reduce_to_number_of_peaks", {"n_max": 1000}),
+               ("require_minimum_of_high_peaks", {"no_peaks": 5, "intensity_percent": 2.0}),
+               ]
+# These filters are in None of the above pipelines
+OTHER_FILTERS = ["require_precursor_below_mz",
+                 "select_by_intensity",
+                 "remove_peaks_around_precursor_mz",
+                 "remove_peaks_outside_top_k",
+                 "require_minimum_number_of_peaks",
+                 "add_fingerprint",
+                 "add_losses",
+                 "repair_parent_mass_match_smiles_wrapper"]
+
+BASIC_FILTERS = HARMONIZE_METADATA_FIELD_NAMES + DERIVE_METADATA_IN_WRONG_FIELD + HARMONIZE_METADATA_ENTRIES
+DEFAULT_FILTERS = BASIC_FILTERS + ["normalize_intensities", ] + REQUIRE_COMPLETE_METADATA + DERIVE_MISSING_METADATA
+FULLY_ANNOTATED_PROCESSING = DEFAULT_FILTERS + REPAIR_ANNOTATION + REQUIRE_COMPLETE_ANNOTATION
+MS2DEEPSCORE_TRAINING = FULLY_ANNOTATED_PROCESSING + CLEAN_PEAKS
+
+PREDEFINED_PIPELINES = {
+    "basic": BASIC_FILTERS,
+    "default": DEFAULT_FILTERS,
+    "fully_annotated": FULLY_ANNOTATED_PROCESSING,
+    "ms2deepscore": MS2DEEPSCORE_TRAINING,
+}
diff --git a/matchms/filtering/filter_order.py b/matchms/filtering/filter_order.py
new file mode 100644
index 00000000..513ec268
--- /dev/null
+++ b/matchms/filtering/filter_order.py
@@ -0,0 +1,53 @@
+from matchms import filtering as msfilters
+
+
+# List all filters in a functionally working order
+
+# IMPORTANT!! IF YOU CHANGE ANYTHING HERE PLEASE ADD A TEST to test_all_filter_order.py
+# to ensure it is not changed back by accident later.
+ALL_FILTERS = [msfilters.make_charge_int,
+               msfilters.add_compound_name,
+               msfilters.derive_adduct_from_name,
+               msfilters.derive_formula_from_name,
+               msfilters.clean_compound_name,
+               msfilters.interpret_pepmass,
+               msfilters.add_precursor_mz,
+               msfilters.add_retention_index,
+               msfilters.add_retention_time,
+               msfilters.derive_ionmode,
+               msfilters.correct_charge,
+               msfilters.require_precursor_mz,
+               msfilters.harmonize_undefined_inchikey,
+               msfilters.harmonize_undefined_inchi,
+               msfilters.harmonize_undefined_smiles,
+               msfilters.repair_inchi_inchikey_smiles,
+               msfilters.clean_adduct,
+               msfilters.add_parent_mass,
+               msfilters.derive_annotation_from_compound_name,
+               msfilters.derive_smiles_from_inchi,
+               msfilters.derive_inchi_from_smiles,
+               msfilters.derive_inchikey_from_inchi,
+               msfilters.repair_smiles_of_salts,
+               msfilters.repair_precursor_is_parent_mass,
+               msfilters.repair_parent_mass_is_mol_wt,
+               msfilters.repair_adduct_based_on_smiles,
+               msfilters.repair_parent_mass_match_smiles_wrapper,
+               msfilters.repair_not_matching_annotation,
+               msfilters.require_valid_annotation,
+               msfilters.require_correct_ionmode,
+               msfilters.require_precursor_below_mz,
+               msfilters.require_parent_mass_match_smiles,
+               msfilters.normalize_intensities,
+               msfilters.select_by_intensity,
+               msfilters.select_by_mz,
+               msfilters.select_by_relative_intensity,
+               msfilters.remove_peaks_around_precursor_mz,
+               msfilters.remove_peaks_outside_top_k,
+               msfilters.reduce_to_number_of_peaks,
+               msfilters.require_minimum_number_of_peaks,
+               msfilters.require_minimum_of_high_peaks,
+               msfilters.add_fingerprint,
+               msfilters.add_losses,
+               ]
+
+FILTER_FUNCTION_NAMES = {x.__name__: x for x in ALL_FILTERS}
diff --git a/matchms/filtering/filter_order_and_default_pipelines.py b/matchms/filtering/filter_order_and_default_pipelines.py
deleted file mode 100644
index 73422c1b..00000000
--- a/matchms/filtering/filter_order_and_default_pipelines.py
+++ /dev/null
@@ -1,105 +0,0 @@
-from matchms import filtering as msfilters
-
-
-# List all filters in a functionally working order
-
-# IMPORTANT!! IF YOU CHANGE ANYTHING HERE PLEASE ADD A TEST to test_all_filter_order.py
-# to ensure it is not changed back by accident later.
-ALL_FILTERS = [msfilters.make_charge_int,
-               msfilters.add_compound_name,
-               msfilters.derive_adduct_from_name,
-               msfilters.derive_formula_from_name,
-               msfilters.clean_compound_name,
-               msfilters.interpret_pepmass,
-               msfilters.add_precursor_mz,
-               msfilters.add_retention_index,
-               msfilters.add_retention_time,
-               msfilters.derive_ionmode,
-               msfilters.correct_charge,
-               msfilters.require_precursor_mz,
-               msfilters.harmonize_undefined_inchikey,
-               msfilters.harmonize_undefined_inchi,
-               msfilters.harmonize_undefined_smiles,
-               msfilters.repair_inchi_inchikey_smiles,
-               msfilters.clean_adduct,
-               msfilters.add_parent_mass,
-               msfilters.derive_annotation_from_compound_name,
-               msfilters.derive_smiles_from_inchi,
-               msfilters.derive_inchi_from_smiles,
-               msfilters.derive_inchikey_from_inchi,
-               msfilters.repair_smiles_of_salts,
-               msfilters.repair_precursor_is_parent_mass,
-               msfilters.repair_parent_mass_is_mol_wt,
-               msfilters.repair_adduct_based_on_smiles,
-               msfilters.repair_parent_mass_match_smiles_wrapper,
-               msfilters.repair_not_matching_annotation,
-               msfilters.require_valid_annotation,
-               msfilters.require_correct_ionmode,
-               msfilters.require_precursor_below_mz,
-               msfilters.require_parent_mass_match_smiles,
-               msfilters.normalize_intensities,
-               msfilters.select_by_intensity,
-               msfilters.select_by_mz,
-               msfilters.select_by_relative_intensity,
-               msfilters.remove_peaks_around_precursor_mz,
-               msfilters.remove_peaks_outside_top_k,
-               msfilters.reduce_to_number_of_peaks,
-               msfilters.require_minimum_number_of_peaks,
-               msfilters.require_minimum_of_high_peaks,
-               msfilters.add_fingerprint,
-               msfilters.add_losses,
-               ]
-FILTER_FUNCTION_NAMES = {x.__name__: x for x in ALL_FILTERS}
-MINIMAL_FILTERS = ["make_charge_int",
-                   "interpret_pepmass",
-                   "derive_ionmode",
-                   "correct_charge",
-                   ]
-BASIC_FILTERS = MINIMAL_FILTERS \
-                + ["add_compound_name",
-                   "derive_adduct_from_name",
-                   "derive_formula_from_name",
-                   "clean_compound_name",
-                   "add_precursor_mz",
-                   ]
-DEFAULT_FILTERS = BASIC_FILTERS \
-                  + ["require_precursor_mz",
-                     "add_parent_mass",
-                     "harmonize_undefined_inchikey",
-                     "harmonize_undefined_inchi",
-                     "harmonize_undefined_smiles",
-                     "repair_inchi_inchikey_smiles",
-                     "normalize_intensities",
-                     "add_retention_time",
-                     ]
-FULLY_ANNOTATED_PROCESSING = DEFAULT_FILTERS \
-                             + ["clean_adduct",
-                                "derive_inchi_from_smiles",
-                                "derive_smiles_from_inchi",
-                                "derive_inchikey_from_inchi",
-                                ("require_correct_ionmode", {"ion_mode_to_keep": "both"}),
-                                ("require_parent_mass_match_smiles", {'mass_tolerance': 0.1}),
-                                ("repair_smiles_of_salts", {'mass_tolerance': 0.1}),
-                                ("repair_precursor_is_parent_mass", {'mass_tolerance': 0.1}),
-                                ("repair_parent_mass_is_mol_wt", {'mass_tolerance': 0.1}),
-                                ("repair_adduct_based_on_smiles", {'mass_tolerance': 0.1}),
-                                "repair_not_matching_annotation",
-                                "require_valid_annotation",
-                                ("derive_annotation_from_compound_name", {"mass_tolerance": 0.1}),
-                                ]
-
-
-MS2DEEPSCORE_TRAINING = FULLY_ANNOTATED_PROCESSING + \
-                        [("select_by_mz", {"mz_from": 0, "mz_to": 1000}),
-                         ("select_by_relative_intensity", {"intensity_from": 0.001}),
-                         ("reduce_to_number_of_peaks", {"n_max": 1000}),
-                         ("require_minimum_of_high_peaks", {"no_peaks": 5, "intensity_percent": 2.0}),
-                         ]
-
-PREDEFINED_PIPELINES = {
-    "minimal": MINIMAL_FILTERS,
-    "basic": BASIC_FILTERS,
-    "default": DEFAULT_FILTERS,
-    "fully_annotated": FULLY_ANNOTATED_PROCESSING,
-    "ms2deepscore": MS2DEEPSCORE_TRAINING,
-}


Test patch:
diff --git a/tests/filtering/test_all_filter_order.py b/tests/filtering/test_all_filter_order.py
index 900b01bd..6613c079 100644
--- a/tests/filtering/test_all_filter_order.py
+++ b/tests/filtering/test_all_filter_order.py
@@ -5,9 +5,7 @@ import os
 from typing import Callable, List
 import pytest
 from matchms import filtering as msfilters
-from matchms.filtering.filter_order_and_default_pipelines import (
-    ALL_FILTERS, PREDEFINED_PIPELINES)
-from matchms.filtering.SpectrumProcessor import SpectrumProcessor
+from matchms.filtering.filter_order import ALL_FILTERS
 
 
 REPAIR_PARENT_MASS_SMILES_FILTERS = \
@@ -95,9 +93,3 @@ def test_all_filters_is_complete():
 def test_all_filters_no_duplicates():
     all_filters = [filter.__name__ for filter in ALL_FILTERS]
     assert len(all_filters) == len(set(all_filters)), "One of the filters appears twice in ALL_FILTERS"
-
-
-def test_create_predefined_pipelines():
-    """Tests if all predefined pipelines can be run"""
-    for pipeline_name in PREDEFINED_PIPELINES:
-        SpectrumProcessor(pipeline_name)
diff --git a/tests/filtering/test_predefined_pipelines.py b/tests/filtering/test_predefined_pipelines.py
new file mode 100644
index 00000000..6188a8d3
--- /dev/null
+++ b/tests/filtering/test_predefined_pipelines.py
@@ -0,0 +1,8 @@
+from matchms import SpectrumProcessor
+from matchms.filtering.default_pipelines import PREDEFINED_PIPELINES
+
+
+def test_create_predefined_pipelines():
+    """Tests if all predefined pipelines can be run"""
+    for pipeline_name in PREDEFINED_PIPELINES:
+        SpectrumProcessor(pipeline_name)
diff --git a/tests/filtering/test_spectrum_processor.py b/tests/filtering/test_spectrum_processor.py
index a8f3e731..f3af1e4f 100644
--- a/tests/filtering/test_spectrum_processor.py
+++ b/tests/filtering/test_spectrum_processor.py
@@ -2,7 +2,7 @@ import os
 import numpy as np
 import pytest
 from matchms import SpectrumProcessor
-from matchms.filtering.filter_order_and_default_pipelines import BASIC_FILTERS
+from matchms.filtering.default_pipelines import BASIC_FILTERS
 from matchms.filtering.SpectrumProcessor import ProcessingReport
 from matchms.importing.load_spectra import load_spectra
 from ..builder_Spectrum import SpectrumBuilder
@@ -23,47 +23,27 @@ 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',
-        'add_retention_time',
-        'derive_ionmode',
-        'correct_charge',
-        'require_precursor_mz',
-        'harmonize_undefined_inchikey',
-        'harmonize_undefined_inchi',
-        'harmonize_undefined_smiles',
-        'repair_inchi_inchikey_smiles',
-        'add_parent_mass',
-        'normalize_intensities'
-    ]
+    processing = SpectrumProcessor(predefined_pipeline=None,
+                                   additional_filters=["make_charge_int",
+                                                       "derive_ionmode",
+                                                       "correct_charge",
+                                                       'derive_adduct_from_name',
+                                                       "interpret_pepmass",
+                                                       ])
+    expected_filters = ['make_charge_int',
+                        'derive_adduct_from_name',
+                        'interpret_pepmass',
+                        'derive_ionmode',
+                        'correct_charge']
 
     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',
-                        'add_retention_time',
                         'derive_ionmode',
-                        'correct_charge',
-                        ('require_precursor_mz', {'minimum_accepted_mz': 10.0}),
-                        ('harmonize_undefined_inchikey', {'aliases': None, 'undefined': ''}),
-                        ('harmonize_undefined_inchi', {'aliases': None, 'undefined': ''}),
-                        ('harmonize_undefined_smiles', {'aliases': None, 'undefined': ''}),
-                        'repair_inchi_inchikey_smiles',
-                        ('add_parent_mass', {'estimate_from_adduct': True, 'overwrite_existing_entry': False}),
-                        'normalize_intensities']
+                        'correct_charge']
     assert processing.processing_steps == expected_filters
 
 
@@ -91,7 +71,12 @@ def test_incomplete_parameters():
 
 
 def test_string_output():
-    processing = SpectrumProcessor("minimal")
+    processing = SpectrumProcessor(predefined_pipeline=None,
+                                   additional_filters=["make_charge_int",
+                                                       "interpret_pepmass",
+                                                       "derive_ionmode",
+                                                       "correct_charge",
+                                                       ])
     expected_str = "Processing steps:\n- make_charge_int\n- interpret_pepmass" \
                    "\n- derive_ionmode\n- correct_charge\n"
     assert str(processing) == expected_str
@@ -104,7 +89,12 @@ def test_string_output():
 ])
 def test_add_matchms_filter(metadata, expected):
     spectrum_in = SpectrumBuilder().with_metadata(metadata).build()
-    processor = SpectrumProcessor("minimal")
+    processor = SpectrumProcessor(predefined_pipeline=None,
+                                   additional_filters=["make_charge_int",
+                                                       "interpret_pepmass",
+                                                       "derive_ionmode",
+                                                       "correct_charge",
+                                                       ])
     processor.add_matchms_filter(("require_correct_ionmode",
                                   {"ion_mode_to_keep": "both"}))
     spectrum = processor.process_spectrum(spectrum_in)
@@ -129,7 +119,12 @@ def test_unknown_keyword():
 
 
 def test_filter_spectrums(spectrums):
-    processor = SpectrumProcessor("minimal")
+    processor = SpectrumProcessor(predefined_pipeline=None,
+                                  additional_filters=["make_charge_int",
+                                                      "interpret_pepmass",
+                                                      "derive_ionmode",
+                                                      "correct_charge",
+                                                     ])
     spectrums, _ = processor.process_spectrums(spectrums)
     assert len(spectrums) == 3
     actual_masses = [s.get("precursor_mz") for s in spectrums]
@@ -138,7 +133,12 @@ def test_filter_spectrums(spectrums):
 
 
 def test_filter_spectrums_report(spectrums):
-    processor = SpectrumProcessor("minimal")
+    processor = SpectrumProcessor(predefined_pipeline=None,
+                                  additional_filters=["make_charge_int",
+                                                      "interpret_pepmass",
+                                                      "derive_ionmode",
+                                                      "correct_charge",
+                                                      ])
     processor.add_filter(filter_function=("require_minimum_number_of_peaks", {"n_required": 2}))
     processor.add_filter(filter_function="add_losses")
     spectrums, report = processor.process_spectrums(spectrums)
@@ -174,7 +174,12 @@ def test_adding_custom_filter(spectrums):
         s_in.set("inchikey", "NONSENSE")
         return s_in
 
-    processor = SpectrumProcessor("minimal")
+    processor = SpectrumProcessor(predefined_pipeline=None,
+                                   additional_filters=["make_charge_int",
+                                                       "interpret_pepmass",
+                                                       "derive_ionmode",
+                                                       "correct_charge",
+                                                       ])
     processor.add_custom_filter(nonsense_inchikey)
     filters = processor.filters
     assert filters[-1].__name__ == "nonsense_inchikey"
@@ -191,7 +196,12 @@ def test_adding_custom_filter_with_parameters(spectrums):
         s_in.set("inchikey", number * "NONSENSE")
         return s_in
 
-    processor = SpectrumProcessor("minimal")
+    processor = SpectrumProcessor(predefined_pipeline=None,
+                                   additional_filters=["make_charge_int",
+                                                       "interpret_pepmass",
+                                                       "derive_ionmode",
+                                                       "correct_charge",
+                                                       ])
     processor.add_custom_filter(nonsense_inchikey_multiple, {"number": 2})
     filters = processor.filters
     assert filters[-1].__name__ == "nonsense_inchikey_multiple"
@@ -216,7 +226,12 @@ def test_add_custom_filter_in_position(filter_position, expected):
         s.set("inchikey", number * "NONSENSE")
         return s
 
-    processor = SpectrumProcessor("minimal")
+    processor = SpectrumProcessor(predefined_pipeline=None,
+                                   additional_filters=["make_charge_int",
+                                                       "interpret_pepmass",
+                                                       "derive_ionmode",
+                                                       "correct_charge",
+                                                       ])
     processor.add_custom_filter(nonsense_inchikey_multiple, {"number": 2},
                                 filter_position=filter_position)
     filters = processor.filters
@@ -229,7 +244,12 @@ def test_add_filter_with_custom(spectrums):
         s.set("inchikey", number * "NONSENSE")
         return s
 
-    processor = SpectrumProcessor("minimal")
+    processor = SpectrumProcessor(predefined_pipeline=None,
+                                  additional_filters=["make_charge_int",
+                                                      "interpret_pepmass",
+                                                      "derive_ionmode",
+                                                      "correct_charge",
+                                                      ])
     processor.add_filter((nonsense_inchikey_multiple, {"number": 2}))
     filters = processor.filters
 
@@ -239,7 +259,12 @@ def test_add_filter_with_custom(spectrums):
 
 
 def test_add_filter_with_matchms_filter(spectrums):
-    processor = SpectrumProcessor("minimal")
+    processor = SpectrumProcessor(predefined_pipeline=None,
+                                  additional_filters=["make_charge_int",
+                                                      "interpret_pepmass",
+                                                      "derive_ionmode",
+                                                      "correct_charge",
+                                                      ])
     processor.add_filter(("require_correct_ionmode",
                           {"ion_mode_to_keep": "both"}))
     filters = processor.filters
@@ -266,7 +291,7 @@ def test_add_filter_twice():
 
 
 def test_save_spectra_spectrum_processor(spectrums, tmp_path):
-    processor = SpectrumProcessor("default")
+    processor = SpectrumProcessor("basic")
     filename = os.path.join(tmp_path, "spectra.msp")
 
     _, _ = processor.process_spectrums(spectrums, cleaned_spectra_file=str(filename))