Repository: matchms/matchms
Pull request #48 (2020-05-26 12:18:44)

Fix wrong ordering of losses and add unit tests
Originally here https://github.com/matchms/matchms-backup/pull/233


-   fix intensity order in ``add_losses``
-   add unit tests
-   add ``loss_mz_from`` and ``loss_mz_to`` parameters to ``add_losses``

Closses #20 

fdiblen: @florian-huber Can you explain what you mean with environment? 
I can think of:
- Python virtual environment (including conda)
- Python environment with different versions of dependencies installed
- Different OS


florian-huber: Didn't do enough testing to really tell exactly. But I would assume it's the second, different versions of dependencies.
cwmeijer: these bugs are in features that are not in matchms. this issue should be in an another repo.
closing
note that you can transfer this issue with a button on the right

Patch:
diff --git a/matchms/filtering/add_losses.py b/matchms/filtering/add_losses.py
index 0a9dd53b..9cb88ae1 100644
--- a/matchms/filtering/add_losses.py
+++ b/matchms/filtering/add_losses.py
@@ -1,8 +1,20 @@
+import numpy
 from ..Spikes import Spikes
+from ..typing import SpectrumType
 
 
-def add_losses(spectrum_in):
-    """Derive losses based on precursor mass."""
+def add_losses(spectrum_in: SpectrumType, loss_mz_from=0.0, loss_mz_to=1000.0) -> SpectrumType:
+    """Derive losses based on precursor mass.
+
+    Args:
+    ----
+    spectrum_in: matchms.Spectrum
+        Input spectrum.
+    loss_mz_from: float
+        Minimum allowed m/z value for losses. Default is 0.0.
+    loss_mz_to: float
+        Maximum allowed m/z value for losses. Default is 1000.0.
+    """
     def precursor_mz_is_number():
         if isinstance(precursor_mz, int):
             return True
@@ -16,11 +28,16 @@ def add_losses(spectrum_in):
     spectrum = spectrum_in.clone()
 
     precursor_mz = spectrum.get("precursor_mz")
-    if precursor_mz is not None:
-        assert precursor_mz_is_number(), "Expected 'precursor_mz' to be a scalar number."
+    if precursor_mz:
+        assert precursor_mz_is_number(), ("Expected 'precursor_mz' to be a scalar number.",
+                                          "Consider applying 'add_precursor_mz' filter first.")
         peaks_mz, peaks_intensities = spectrum.peaks
         losses_mz = (precursor_mz - peaks_mz)[::-1]
-        losses_intensities = peaks_intensities
-        spectrum.losses = Spikes(mz=losses_mz, intensities=losses_intensities)
+        losses_intensities = peaks_intensities[::-1]
+        # Add losses which are within given boundaries
+        mask = numpy.where((losses_mz >= loss_mz_from)
+                           & (losses_mz <= loss_mz_to))
+        spectrum.losses = Spikes(mz=losses_mz[mask],
+                                 intensities=losses_intensities[mask])
 
     return spectrum


Test patch:
diff --git a/tests/test_add_losses.py b/tests/test_add_losses.py
index a9bb1d23..6fdd73f3 100644
--- a/tests/test_add_losses.py
+++ b/tests/test_add_losses.py
@@ -5,16 +5,21 @@ from matchms.filtering import add_losses
 
 
 def test_add_losses():
+    """Test if all losses are correctly generated form mz values and precursor-m/z."""
     spectrum_in = Spectrum(mz=numpy.array([100, 150, 200, 300], dtype="float"),
                            intensities=numpy.array([700, 200, 100, 1000], dtype="float"),
                            metadata={"precursor_mz": 445.0})
 
     spectrum = add_losses(spectrum_in)
 
-    assert numpy.allclose(spectrum.losses.mz, numpy.array([145, 245, 295, 345], "float"))
+    expected_mz = numpy.array([145, 245, 295, 345], "float")
+    expected_intensities = numpy.array([1000, 100, 200, 700], "float")
+    assert numpy.allclose(spectrum.losses.mz, expected_mz), "Expected different loss m/z."
+    assert numpy.allclose(spectrum.losses.intensities, expected_intensities), "Expected different intensities."
 
 
 def test_add_losses_without_precursor_mz():
+    """Test if no changes are done without having a precursor-m/z."""
     spectrum_in = Spectrum(mz=numpy.array([100, 150, 200, 300], dtype="float"),
                            intensities=numpy.array([700, 200, 100, 1000], dtype="float"))
 
@@ -24,7 +29,7 @@ def test_add_losses_without_precursor_mz():
 
 
 def test_add_losses_with_precursor_mz_wrong_type():
-
+    """Test if correct assert error is raised for precursor-mz as string."""
     spectrum_in = Spectrum(mz=numpy.array([100, 150, 200, 300], dtype="float"),
                            intensities=numpy.array([700, 200, 100, 1000], dtype="float"),
                            metadata={"precursor_mz": "445.0"})
@@ -32,10 +37,11 @@ def test_add_losses_with_precursor_mz_wrong_type():
     with pytest.raises(AssertionError) as msg:
         _ = add_losses(spectrum_in)
 
-    assert str(msg.value) == "Expected 'precursor_mz' to be a scalar number."
+    assert "Expected 'precursor_mz' to be a scalar number." in str(msg.value)
 
 
 def test_add_losses_returns_new_spectrum_instance():
+    """Test if no change is done to empty spectrum."""
     spectrum_in = Spectrum(mz=numpy.array([], dtype="float"),
                            intensities=numpy.array([], dtype="float"))
 
@@ -45,6 +51,35 @@ def test_add_losses_returns_new_spectrum_instance():
 
 
 def test_add_losses_with_input_none():
+    """Test if input spectrum is None."""
     spectrum_in = None
     spectrum = add_losses(spectrum_in)
     assert spectrum is None
+
+
+def test_add_losses_with_peakmz_larger_precursormz():
+    """Test if losses are correctly generated and loss < 0 is discarded."""
+    spectrum_in = Spectrum(mz=numpy.array([100, 150, 200, 450], dtype="float"),
+                           intensities=numpy.array([700, 200, 100, 1000], dtype="float"),
+                           metadata={"precursor_mz": 445.0})
+
+    spectrum = add_losses(spectrum_in)
+
+    expected_mz = numpy.array([245, 295, 345], "float")
+    expected_intensities = numpy.array([100, 200, 700], "float")
+    assert numpy.allclose(spectrum.losses.mz, expected_mz), "Expected different loss m/z."
+    assert numpy.allclose(spectrum.losses.intensities, expected_intensities), "Expected different intensities."
+
+
+def test_add_losses_with_max_loss_mz_250():
+    """Test if losses are correctly generated and losses with mz > 250 are discarded."""
+    spectrum_in = Spectrum(mz=numpy.array([100, 150, 200, 300], dtype="float"),
+                           intensities=numpy.array([700, 200, 100, 1000], dtype="float"),
+                           metadata={"precursor_mz": 445.0})
+
+    spectrum = add_losses(spectrum_in, loss_mz_to=250)
+
+    expected_mz = numpy.array([145, 245], "float")
+    expected_intensities = numpy.array([1000, 100], "float")
+    assert numpy.allclose(spectrum.losses.mz, expected_mz), "Expected different loss m/z."
+    assert numpy.allclose(spectrum.losses.intensities, expected_intensities), "Expected different intensities."