Repository: matchms/matchms
Pull request #671 (2024-06-03 08:55:03)

Fix saving bug
Saving with spectrum processor had a bug introduced in #651 
When a spectrum is set to None it would still try to save it resulting in the pipeline breaking.

Maybe we should also add the ability to handle None values in the general save functions, but for now a quick fix.

@tornikeo fyi


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 09162f22..400760a3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,12 @@ 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).
 
+## [0.26.0] -2024-06-03
+### Changed
+- Fixed bug. Removing spectra in spectrum processor would break the saving, since trying to save None values.
+
+## [0.26.0] -2024-06-03
+
 ## Unreleased
 ### Added
 - Added remove_profile_spectra filter
diff --git a/matchms/filtering/SpectrumProcessor.py b/matchms/filtering/SpectrumProcessor.py
index c7aebf9e..8de7defd 100644
--- a/matchms/filtering/SpectrumProcessor.py
+++ b/matchms/filtering/SpectrumProcessor.py
@@ -203,8 +203,8 @@ class SpectrumProcessor:
             if processed_spectrum is not None:
                 processed_spectrums.append(processed_spectrum)
 
-            if cleaned_spectra_file is not None and incremental_save:
-                save_spectra(processed_spectrum, cleaned_spectra_file, append=True)
+                if cleaned_spectra_file is not None and incremental_save:
+                    save_spectra(processed_spectrum, cleaned_spectra_file, append=True)
 
         if cleaned_spectra_file is not None and not incremental_save:
             save_spectra(processed_spectrums, cleaned_spectra_file)


Test patch:
diff --git a/tests/filtering/test_spectrum_processor.py b/tests/filtering/test_spectrum_processor.py
index 9dd9dd34..e084105e 100644
--- a/tests/filtering/test_spectrum_processor.py
+++ b/tests/filtering/test_spectrum_processor.py
@@ -11,14 +11,14 @@ from ..builder_Spectrum import SpectrumBuilder
 
 @pytest.fixture
 def spectrums():
-    s1 = SpectrumBuilder().\
-        with_metadata({"charge": "+1", "pepmass": 100}).\
+    s1 = SpectrumBuilder(). \
+        with_metadata({"charge": "+1", "pepmass": 100}). \
         with_mz([10, 20, 30]).with_intensities([0.1, 0.4, 10]).build()
     s2 = SpectrumBuilder().with_metadata({"charge": "-1",
-                                          "pepmass": 102}).\
+                                          "pepmass": 102}). \
         with_mz([10, 20, 30]).with_intensities([0.1, 0.2, 1]).build()
     s3 = SpectrumBuilder().with_metadata({"charge": -1,
-                                          "pepmass": 104}).\
+                                          "pepmass": 104}). \
         with_mz([10, ]).with_intensities([0.1, ]).build()
     return [s1, s2, s3]
 
@@ -191,6 +191,7 @@ def test_adding_custom_filter_with_parameters(spectrums):
 ])
 def test_add_custom_filter_in_position(filter_position: int, expected):
     """Tests that a filter is added in the correct position"""
+
     def nonsense_inchikey_multiple(s, number):
         s.set("inchikey", number * "NONSENSE")
         return s
@@ -305,6 +306,7 @@ def test_add_all_filter_types(spectrums):
     spectrums, _ = processor.process_spectrums(spectrums)
     assert spectrums[0].get("inchikey") == "NONSENSENONSENSE", "Custom filter not executed properly"
 
+
 def test_save_spectra_spectrum_processor(spectrums, tmp_path):
     processor = SpectrumProcessor(BASIC_FILTERS)
     filename = os.path.join(tmp_path, "spectra.msp")
@@ -320,10 +322,28 @@ def test_save_spectra_spectrum_processor(spectrums, tmp_path):
     for spectrum in reloaded_spectra:
         assert spectrum.get("precursor_mz") is not None
 
+
+def test_save_spectra_spectrum_processor_none_spectra(spectrums, tmp_path):
+    processor = SpectrumProcessor(BASIC_FILTERS +
+                                  [(msfilters.require_correct_ionmode, {"ion_mode_to_keep": "positive"})])
+    filename = os.path.join(tmp_path, "spectra.msp")
+
+    _, _ = processor.process_spectrums(spectrums, cleaned_spectra_file=str(filename))
+    assert os.path.exists(filename)
+
+    # Reload spectra and compare lengths
+    reloaded_spectra = list(load_spectra(str(filename)))
+    assert len(reloaded_spectra) == 1
+
+    # Check that the processed spectra are stored
+    for spectrum in reloaded_spectra:
+        assert spectrum.get("precursor_mz") is not None
+
+
 @pytest.mark.parametrize('ftype, should_work', [
-    ['msp',True], 
-    ['mgf',True],
-    ['json',False],
+    ['msp', True],
+    ['mgf', True],
+    ['json', False],
 ])
 def test_save_partial_spectra_spectrum_processor(ftype, should_work, spectrums, tmp_path):
     processor = SpectrumProcessor(BASIC_FILTERS)
@@ -332,16 +352,16 @@ def test_save_partial_spectra_spectrum_processor(ftype, should_work, spectrums,
     spectrums[-2] = "Lorem Ipsum"
 
     filename = os.path.join(tmp_path, f"spectra.{ftype}")
-    
+
     with pytest.raises(Exception):
         _, _ = processor.process_spectrums(spectrums, cleaned_spectra_file=str(filename))
-    
+
     if should_work:
         assert os.path.exists(filename)
 
         # Reload spectra and compare lengths
         reloaded_spectra = list(load_spectra(str(filename)))
-        
+
         # Make sure we are only missing last two "spectra"
         assert len(reloaded_spectra) == len(spectrums) - 2
 
@@ -349,4 +369,4 @@ def test_save_partial_spectra_spectrum_processor(ftype, should_work, spectrums,
         for spectrum in reloaded_spectra:
             assert spectrum.get("precursor_mz") is not None
     else:
-        assert not os.path.exists(filename)
\ No newline at end of file
+        assert not os.path.exists(filename)