Repository: matchms/matchms
Pull request #481 (2023-08-14 11:58:16)

Remove file names from yaml
Currently the yaml file stores the query and reference files.
This is not intuitive to me when sharing pipelines, since a path will always be different. To me it would make more sense to have a pipeline file store all the steps, but specifying the query files and reference files in the python script were Pipeline is defined. Instead the log file should store the query file and reference file location. 

- [x] Remove spectrum files from yaml file
- [x] Add spectrum files to logging
- [x] Specify file names in run, instead of as Pipeline attributes

(Docstring will be updated, once the final setup is selected)


Patch:
diff --git a/.github/workflows/CI_build.yml b/.github/workflows/CI_build.yml
index f8e3c7cd..09a25c89 100644
--- a/.github/workflows/CI_build.yml
+++ b/.github/workflows/CI_build.yml
@@ -1,8 +1,9 @@
 name: CI Build
 
 on:
+  push:
   pull_request:
-    types: [opened, ready_for_review, closed]
+    branches: [ master ]
   workflow_dispatch:
     inputs:
       tag:
@@ -11,6 +12,7 @@ on:
         default: ''
 
 jobs:
+
   first_check:
     name: first code check / python-3.9 / ubuntu-latest
     runs-on: ubuntu-latest
diff --git a/.github/workflows/CI_first_code_check.yml b/.github/workflows/CI_first_code_check.yml
deleted file mode 100644
index d667c4bb..00000000
--- a/.github/workflows/CI_first_code_check.yml
+++ /dev/null
@@ -1,60 +0,0 @@
-name: CI first code check
-
-on:
-  push:
-  workflow_dispatch:
-    inputs:
-      tag:
-        description: Tag for manually running CI first code check workflow
-        required: False
-        default: ''
-
-jobs:
-
-  first_check:
-    name: first code check / python-3.9 / ubuntu-latest
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v3
-      - name: Set up Python
-        uses: actions/setup-python@v2
-        with:
-          python-version: 3.9
-      - name: Python info
-        run: |
-          which python
-          python --version
-      - name: Build package and create dev environment
-        run: |
-          python -m pip install --upgrade pip poetry
-          poetry install
-      - name: Show pip list
-        run: |
-          pip list
-      - name: Test with coverage
-        run: |
-          poetry run pytest --cov --cov-report term --cov-report xml --junitxml=xunit-result.xml
-      - name: Correct coverage paths
-        run: sed -i "s+$PWD/++g" coverage.xml
-      - name: Check style against standards using prospector
-        shell: bash -l {0}
-        run: prospector -o grouped -o pylint:pylint-report.txt
-      - name: Store PR metadata in json file
-        env:
-          PR_NUMBER: ${{ github.event.pull_request.number }}
-          HEAD_REF: ${{ github.event.pull_request.head.ref }}
-          BASE_REF: ${{ github.event.pull_request.base.ref }}
-        run: |
-          jq -n --arg pr_number "$PR_NUMBER"\
-                --arg head_ref "$HEAD_REF"\
-                --arg base_ref "$BASE_REF"\
-            '{"pr_number": $pr_number, "head_ref": $head_ref, "base_ref": $base_ref}' > pr_metadata.json
-      - uses: actions/upload-artifact@v3
-        with:
-          name: sonarcloud-data-pip
-          path: |
-            coverage.xml
-            xunit-result.xml
-            pr_metadata.json
-      - name: Check whether import statements are used consistently
-        run: isort . --check-only --diff
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 76da7629..32b7b238 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,16 +5,6 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [unreleased]
-
-### 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.
-
-### Changed
-
-- Extensive code refactoring and cleaning.
-
 ## [0.21.2] - 2023-08-01
 
 ### Added
diff --git a/NOTICE b/NOTICE
new file mode 100644
index 00000000..e69de29b
diff --git a/README.rst b/README.rst
index 3ebdd02e..0250c3bb 100644
--- a/README.rst
+++ b/README.rst
@@ -137,8 +137,6 @@ To date we are aware of:
 
 + `matchmsextras <https://github.com/matchms/matchmsextras>`_ which contains additional functions to create networks based on spectral similarities, to run spectrum searchers against `PubChem`, or additional plotting methods.
 
-+ `MS2Query <https://github.com/iomega/ms2query>`_ Reliable and fast MS/MS spectral-based analogue search, running on top op matchms.
-
 + `memo <https://github.com/mandelbrot-project/memo>`_ a method allowing a Retention Time (RT) agnostic alignment of metabolomics samples using the fragmentation spectra (MS2) of their consituents.
 
 *(if you know of any other packages that are fully compatible with matchms, let us know!)*
diff --git a/matchms/Pipeline.py b/matchms/Pipeline.py
index e8959884..f7f15a31 100644
--- a/matchms/Pipeline.py
+++ b/matchms/Pipeline.py
@@ -23,13 +23,9 @@ def create_workflow(yaml_file_name=None,
                     additional_filters_queries=(),
                     additional_filters_references=(),
                     score_computations=(),
-                    query_file_name=None,
-                    reference_file_name=None,
                     ):
     # pylint: disable=too-many-arguments
     workflow = OrderedDict()
-    workflow["importing"] = {"queries": query_file_name,
-                             "references": reference_file_name}
     queries_processor = initialize_spectrum_processor(predefined_processing_queries, additional_filters_queries)
     workflow["query_filters"] = queries_processor.processing_steps
     queries_processor = initialize_spectrum_processor(predefined_processing_reference, additional_filters_references)
@@ -141,26 +137,29 @@ class Pipeline:
                                        ["filter_by_range", {"name": "Spec2Vec", "low": 0.3}]]
 
     """
-    def __init__(self, workflow: OrderedDict, progress_bar=True):
+    def __init__(self, workflow, progress_bar=True):
         """
         Parameters
         ----------
-        workflow:
-            Contains an orderedDict containing the workflow settings. Can be created using create_workflow.
-        progress_bar:
+        config_file
+            Filename of config file (yaml file) to define pipeline. Default is None
+            in which case the pipeline should be defined via a Python script.
+        progress_bar
             Default is True. Set to False if no progress bar should be displayed.
         """
-        self.spectrums_queries = []
-        self.spectrums_references = []
+        self._spectrums_queries = None
+        self._spectrums_references = None
         self.is_symmetric = False
+        self.scores = None
+
+        self.progress_bar = progress_bar
         self.workflow = workflow
         self.complete_workflow()
+
         self.write_to_logfile("--- Processing pipeline: ---")
         self._initialize_spectrum_processor_queries()
         if self.is_symmetric is False:
             self._initialize_spectrum_processor_references()
-        self.scores = None
-        self.progress_bar = progress_bar
 
     def _initialize_spectrum_processor_queries(self):
         """Initialize spectrum processing workflow for the query spectra."""
@@ -197,7 +196,7 @@ class Pipeline:
         if self.logging_level is None:
             self.logging_level = "WARNING"
 
-    def run(self):
+    def run(self, query_files, reference_files = None):
         """Execute the defined Pipeline workflow.
 
         This method will execute all steps of the workflow.
@@ -209,29 +208,28 @@ class Pipeline:
         self.write_to_logfile("--- Start running matchms pipeline. ---")
         self.write_to_logfile(f"Start time: {str(datetime.now())}")
         self.check_pipeline()
-        self.write_to_logfile("--- Importing data ---")
-        self.import_data(self.query_files,
-                         self.reference_files)
+        self.import_data(query_files,
+                         reference_files)
 
         # Processing
         self.write_to_logfile("--- Processing spectra ---")
         self.write_to_logfile(f"Time: {str(datetime.now())}")
         # Process query spectra
         spectrums, report = self.processing_queries.process_spectrums(
-            self.spectrums_queries,
+            self._spectrums_queries,
             create_report=True,
             progress_bar=self.progress_bar)
-        self.spectrums_queries = spectrums
+        self._spectrums_queries = spectrums
         self.write_to_logfile(str(report))
         # Process reference spectra (if necessary)
         if self.is_symmetric is False:
-            self.spectrums_references, report = self.processing_references.process_spectrums(
-                self.spectrums_references,
+            self._spectrums_references, report = self.processing_references.process_spectrums(
+                self._spectrums_references,
                 create_report=True,
                 progress_bar=self.progress_bar)
             self.write_to_logfile(str(report))
         else:
-            self.spectrums_references = self.spectrums_queries
+            self._spectrums_references = self._spectrums_queries
 
         # Score computation and masking
         self.write_to_logfile("--- Computing scores ---")
@@ -275,14 +273,14 @@ class Pipeline:
         similarity_measure = get_similarity_measure(computation)
         # If this is the first score computation:
         if i == 0:
-            self.scores = calculate_scores(self.spectrums_references,
-                                           self.spectrums_queries,
+            self.scores = calculate_scores(self._spectrums_references,
+                                           self._spectrums_queries,
                                            similarity_measure,
                                            array_type="sparse",
                                            is_symmetric=self.is_symmetric)
         else:
-            new_scores = similarity_measure.sparse_array(references=self.spectrums_references,
-                                                         queries=self.spectrums_queries,
+            new_scores = similarity_measure.sparse_array(references=self._spectrums_references,
+                                                         queries=self._spectrums_queries,
                                                          idx_row=self.scores.scores.row,
                                                          idx_col=self.scores.scores.col,
                                                          is_symmetric=self.is_symmetric)
@@ -295,17 +293,6 @@ class Pipeline:
         """Check if pipeline seems OK before running.
         Aim is to avoid pipeline crashing after long computation.
         """
-        def check_files_exist(filenames):
-            if isinstance(filenames, str):
-                filenames = [filenames]
-            for filename in filenames:
-                assert os.path.exists(filename), f"File {filename} not found."
-
-        # Check if all files exist
-        check_files_exist(self.query_files)
-        if self.reference_files is not None:
-            check_files_exist(self.reference_files)
-
         # Check if all score compuation steps exist
         for computation in self.score_computations:
             if not isinstance(computation, list):
@@ -329,8 +316,8 @@ class Pipeline:
                                 remove_stream_handlers=True)
         else:
             set_matchms_logger_level(self.logging_level)
-            logger.warning("No logging file was defined." \
-                "Logging messages will not be written to file.")
+            logger.warning("No logging file was defined."
+                           "Logging messages will not be written to file.")
 
     def write_to_logfile(self, line):
         """Write message to log file.
@@ -350,41 +337,46 @@ class Pipeline:
             List of files, or single filename, containing the reference spectra.
             If set to None (default) then all query spectra will be compared to each other.
         """
+        def check_files_exist(filenames):
+            assert filenames is not None
+            if isinstance(filenames, str):
+                filenames = [filenames]
+            for filename in filenames:
+                assert os.path.exists(filename), f"File {filename} not found."
+
+        # Check if all files exist
+        check_files_exist(query_files)
+        if reference_files is not None:
+            check_files_exist(reference_files)
+
         if isinstance(query_files, str):
             query_files = [query_files]
         if isinstance(reference_files, str):
             reference_files = [reference_files]
+        self.write_to_logfile("--- Importing data ---")
+
+        # import query spectra
         spectrums_queries = []
         for query_file in query_files:
             spectrums_queries += list(load_spectra(query_file))
-        self.spectrums_queries += spectrums_queries
+        self._spectrums_queries = spectrums_queries
+        self.write_to_logfile(f"Loaded query spectra from {query_files}")
+        self.write_to_logfile(f"Loaded {len(self._spectrums_queries)} query spectra")
+
+        # import reference spectra
         if reference_files is None:
             self.is_symmetric = True
-            self.spectrums_references = self.spectrums_queries
+            self._spectrums_references = self._spectrums_queries
+            self.write_to_logfile("Reference spectra are equal to the query spectra (is_symmetric = True)")
         else:
             spectrums_references = []
             for reference_file in reference_files:
                 spectrums_references += list(load_spectra(reference_file))
-            self.spectrums_references += spectrums_references
-
+            self._spectrums_references = spectrums_references
+            self.write_to_logfile(f"Loaded reference spectra from {reference_files}")
+            self.write_to_logfile(f"Loaded {len(self._spectrums_references)} reference spectra")
 
     # Getter & Setters
-    @property
-    def query_files(self):
-        return self.workflow["importing"].get("queries")
-
-    @query_files.setter
-    def query_files(self, filter_list):
-        self.workflow["importing"]["queries"] = filter_list
-
-    @property
-    def reference_files(self):
-        return self.workflow["importing"].get("references")
-
-    @reference_files.setter
-    def reference_files(self, files):
-        self.workflow["importing"]["references"] = files
-
     @property
     def logging_file(self):
         return self.workflow["logging"].get("logging_file")
@@ -408,3 +400,11 @@ class Pipeline:
     @score_computations.setter
     def score_computations(self, computations_list):
         self.workflow["score_computations"] = computations_list
+
+    @property
+    def spectrums_queries(self):
+        return self._spectrums_queries
+
+    @property
+    def spectrums_references(self):
+        return self._spectrums_references


Test patch:
diff --git a/integration-tests/test_user_workflow.py b/integration-tests/test_user_workflow.py
index 35c7210f..0e128fdf 100644
--- a/integration-tests/test_user_workflow.py
+++ b/integration-tests/test_user_workflow.py
@@ -8,9 +8,7 @@ from matchms.similarity import CosineGreedy
 
 def test_user_workflow():
     module_root = os.path.join(os.path.dirname(__file__), "..")
-    spectrums_file = os.path.join(module_root, "tests", "testdata", "pesticides.mgf")
-    workflow = create_workflow(query_file_name=spectrums_file,
-                               predefined_processing_queries="basic",
+    workflow = create_workflow(predefined_processing_queries="basic",
                                additional_filters_queries=[["add_parent_mass"],
                                                            ["normalize_intensities"],
                                                            ["select_by_relative_intensity", {"intensity_from": 0.0, "intensity_to": 1.0}],
@@ -18,7 +16,8 @@ def test_user_workflow():
                                                            ["require_minimum_number_of_peaks", {"n_required": 5}]],
                                score_computations=[["cosinegreedy",  {"tolerance": 0.3}]])
     pipeline = Pipeline(workflow)
-    pipeline.run()
+    spectrums_file = os.path.join(module_root, "tests", "testdata", "pesticides.mgf")
+    pipeline.run(spectrums_file)
 
     scores = pipeline.scores
 
diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py
index 8fe47c5f..9fe3ec98 100644
--- a/tests/test_pipeline.py
+++ b/tests/test_pipeline.py
@@ -14,31 +14,28 @@ spectrums_file_msp = os.path.join(module_root, "tests", "testdata", "massbank_fi
 def test_pipeline_initial_check_missing_file():
     workflow = create_workflow()
     pipeline = Pipeline(workflow)
-    pipeline.query_files = "non_existing_file.msp"
     pipeline.score_computations = [["precursormzmatch",  {"tolerance": 120.0}]]
     with pytest.raises(AssertionError) as msg:
-        pipeline.run()
+        pipeline.run("non_existing_file.msp")
     assert "not found" in str(msg.value)
 
 
 def test_pipeline_initial_check_unknown_step():
     workflow = create_workflow()
     pipeline = Pipeline(workflow)
-    pipeline.query_files = spectrums_file_msp
     pipeline.score_computations = [["precursormzOOPSmatch",  {"tolerance": 120.0}]]
     with pytest.raises(ValueError) as msg:
-        pipeline.run()
+        pipeline.run(spectrums_file_msp)
     assert "Unknown score computation:" in str(msg.value)
 
 
 def test_pipeline_symmetric():
     workflow = create_workflow()
     pipeline = Pipeline(workflow)
-    pipeline.query_files = spectrums_file_msp
     pipeline.predefined_processing_queries = "basic"
     pipeline.score_computations = [["precursormzmatch",  {"tolerance": 120.0}],
                                    ["modifiedcosine", {"tolerance": 10.0}]]
-    pipeline.run()
+    pipeline.run(spectrums_file_msp)
 
     assert len(pipeline.spectrums_queries) == 5
     assert pipeline.spectrums_queries[0] == pipeline.spectrums_references[0]
@@ -55,12 +52,11 @@ def test_pipeline_symmetric():
 def test_pipeline_symmetric_filters():
     workflow = create_workflow()
     pipeline = Pipeline(workflow)
-    pipeline.query_files = spectrums_file_msp
     pipeline.predefined_processing_queries = "basic"
     pipeline.additional_processing_queries = [[select_by_mz, {"mz_from": 0, "mz_to": 1000}]]
     pipeline.score_computations = [["precursormzmatch",  {"tolerance": 120.0}],
                                    ["modifiedcosine", {"tolerance": 10.0}]]
-    pipeline.run()
+    pipeline.run(spectrums_file_msp)
 
     assert len(pipeline.spectrums_queries) == 5
     assert pipeline.spectrums_queries[0].metadata == pipeline.spectrums_references[0].metadata
@@ -78,12 +74,11 @@ def test_pipeline_symmetric_filters():
 def test_pipeline_symmetric_masking():
     workflow = create_workflow()
     pipeline = Pipeline(workflow)
-    pipeline.query_files = spectrums_file_msp
     pipeline.predefined_processing_queries = "basic"
     pipeline.score_computations = [["precursormzmatch",  {"tolerance": 120.0}],
                                    ["modifiedcosine", {"tolerance": 10.0}],
                                    ["filter_by_range", {"low": 0.3, "above_operator": '>='}]]
-    pipeline.run()
+    pipeline.run(spectrums_file_msp)
 
     assert len(pipeline.spectrums_queries) == 5
     assert pipeline.spectrums_queries[0] == pipeline.spectrums_references[0]
@@ -100,11 +95,10 @@ def test_pipeline_symmetric_masking():
 def test_pipeline_symmetric_custom_score():
     workflow = create_workflow()
     pipeline = Pipeline(workflow)
-    pipeline.query_files = spectrums_file_msp
     pipeline.predefined_processing_queries = "basic"
     pipeline.score_computations = [["precursormzmatch",  {"tolerance": 120.0}],
                                    [ModifiedCosine, {"tolerance": 10.0}]]
-    pipeline.run()
+    pipeline.run(spectrums_file_msp)
 
     assert len(pipeline.spectrums_queries) == 5
     assert pipeline.spectrums_queries[0] == pipeline.spectrums_references[0]
@@ -122,13 +116,11 @@ def test_pipeline_non_symmetric():
     """Test importing from multiple files and different inputs for query and references."""
     workflow = create_workflow()
     pipeline = Pipeline(workflow)
-    pipeline.query_files = spectrums_file_msp
     pipeline.predefined_processing_queries = "basic"
     pipeline.predefined_processing_references = "basic"
-    pipeline.reference_files = [spectrums_file_msp, spectrums_file_msp]
     pipeline.score_computations = [["precursormzmatch",  {"tolerance": 120.0}],
                                    ["modifiedcosine", {"tolerance": 10.0}]]
-    pipeline.run()
+    pipeline.run(spectrums_file_msp, [spectrums_file_msp, spectrums_file_msp])
 
     assert len(pipeline.spectrums_queries) == 5
     assert len(pipeline.spectrums_references) == 10
@@ -148,7 +140,7 @@ def test_pipeline_from_yaml():
     config_file = os.path.join(module_root, "tests", "test_pipeline.yaml")
     workflow = load_in_workflow_from_yaml_file(config_file)
     pipeline = Pipeline(workflow)
-    pipeline.run()
+    pipeline.run(spectrums_file_msp)
     assert len(pipeline.spectrums_queries) == 5
     assert pipeline.spectrums_queries[0] == pipeline.spectrums_references[0]
     assert pipeline.is_symmetric is True
@@ -165,18 +157,17 @@ def test_pipeline_to_and_from_yaml(tmp_path):
     config_file = os.path.join(tmp_path, "test_pipeline.yaml")
 
     workflow = create_workflow(config_file, score_computations=[["precursormzmatch",  {"tolerance": 120.0}],
-                                                                ["modifiedcosine", {"tolerance": 10.0}]],
-                               query_file_name=spectrums_file_msp)
+                                                                ["modifiedcosine", {"tolerance": 10.0}]])
     assert os.path.exists(config_file)
 
     pipeline = Pipeline(workflow)
-    pipeline.run()
+    pipeline.run(spectrums_file_msp)
     scores_run1 = pipeline.scores
 
     # Load again
     workflow = load_in_workflow_from_yaml_file(config_file)
     pipeline = Pipeline(workflow)
-    pipeline.run()
+    pipeline.run(spectrums_file_msp)
     assert pipeline.scores.scores == scores_run1.scores
 
 
@@ -184,12 +175,9 @@ def test_pipeline_logging(tmp_path):
     pytest.importorskip("rdkit")
     workflow = create_workflow()
     pipeline = Pipeline(workflow)
-    pipeline.query_files = spectrums_file_msp
-    pipeline.score_computations = [["precursormzmatch",  {"tolerance": 120.0}],
-                                   [ModifiedCosine, {"tolerance": 10.0}]]
     logfile = os.path.join(tmp_path, "testlog.log")
     pipeline.logging_file = logfile
-    pipeline.run()
+    pipeline.run(spectrums_file_msp)
 
     assert os.path.exists(logfile)
     with open(logfile, "r", encoding="utf-8") as f:
@@ -203,13 +191,11 @@ def test_FingerprintSimilarity_pipeline():
                                additional_filters_queries=["add_fingerprint"],
                                predefined_processing_reference="basic",
                                additional_filters_references=["add_fingerprint"],
-                               query_file_name=spectrums_file_msp,
-                               reference_file_name=spectrums_file_msp,
                                score_computations=[["metadatamatch", {"field": "precursor_mz", "matching_type": "difference", "tolerance": 50}],
                                                    ["fingerprintsimilarity", {"similarity_measure": "jaccard"}]],
                                )
     pipeline = Pipeline(workflow)
-    pipeline.run()
+    pipeline.run(spectrums_file_msp, spectrums_file_msp)
     assert len(pipeline.spectrums_queries[0].get("fingerprint")) == 2048
     assert pipeline.scores.scores.shape == (5, 5, 2)
     assert pipeline.scores.score_names == ('MetadataMatch', 'FingerprintSimilarity')
diff --git a/tests/test_pipeline.yaml b/tests/test_pipeline.yaml
index 1a3ca9b9..e70e169a 100644
--- a/tests/test_pipeline.yaml
+++ b/tests/test_pipeline.yaml
@@ -1,9 +1,6 @@
 # Matchms pipeline config file
 # Change and adapt fields where necessary
 # ====================
-importing:
-  queries: ./tests/testdata/massbank_five_spectra.msp
-  references:
 query_filters:
 - - make_charge_int
 - - interpret_pepmass
diff --git a/tests/test_scoring_method_consistency.py b/tests/test_scoring_method_consistency.py
index 77f4ccc5..cd6d22e0 100644
--- a/tests/test_scoring_method_consistency.py
+++ b/tests/test_scoring_method_consistency.py
@@ -84,13 +84,12 @@ def test_consistency_scoring_and_pipeline(spectrums, similarity_measure):
     computed_scores_matrix = scoring_method.matrix(spectrums, spectrums)
 
     # Run pipeline
-    workflow = create_workflow(query_file_name=json_file,
-                               predefined_processing_queries="basic",
+    workflow = create_workflow(predefined_processing_queries="basic",
                                additional_filters_queries=[["add_parent_mass"], ["normalize_intensities"]],
                                score_computations=[similarity_measure]
                                )
     pipeline = Pipeline(workflow)
-    pipeline.run()
+    pipeline.run(json_file)
 
     if computed_scores_matrix.dtype.names is None:
         assert np.allclose(pipeline.scores.to_array(), computed_scores_matrix)