Repository: matchms/matchms
Pull request #191 (2021-03-03 17:04:33)

Added require_precursor_mz_above_0 and tests

sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!

[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=BUG)  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=SECURITY_HOTSPOT) to review)  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=CODE_SMELL)

[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=matchms_matchms&pullRequest=191&metric=coverage&view=list) No Coverage information  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=matchms_matchms&pullRequest=191&metric=duplicated_lines_density&view=list) No Duplication information


fdiblen: As @sverhoeven mentioned before official numpy examples use np.
See https://numpy.org/devdocs/user/quickstart.html#an-example.

I suggest we merge this asap and make sure that we always use what we decide here.
sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!

[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=BUG)  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=SECURITY_HOTSPOT) to review)  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=CODE_SMELL)

[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=matchms_matchms&pullRequest=191&metric=coverage&view=list) No Coverage information  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=matchms_matchms&pullRequest=191&metric=duplicated_lines_density&view=list) No Duplication information


sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!

[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=BUG)  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=SECURITY_HOTSPOT) to review)  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=191&resolved=false&types=CODE_SMELL)

[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=matchms_matchms&pullRequest=191&metric=coverage&view=list) No Coverage information  
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=matchms_matchms&pullRequest=191&metric=duplicated_lines_density&view=list) No Duplication information



Patch:
diff --git a/.zenodo.json b/.zenodo.json
index 74921c46..599436c1 100644
--- a/.zenodo.json
+++ b/.zenodo.json
@@ -1,76 +1,81 @@
 {
    "creators": [
       {
-         "affiliation": "Netherlands eScience Center", 
-         "name": "Huber, Florian", 
+         "affiliation": "Netherlands eScience Center",
+         "name": "Huber, Florian",
          "orcid": "0000-0002-3535-9406"
-      }, 
+      },
       {
-         "affiliation": "Wageningen University and Research", 
-         "name": "van der Hooft, Justin J. J.", 
+         "affiliation": "Wageningen University and Research",
+         "name": "van der Hooft, Justin J. J.",
          "orcid": "0000-0002-9340-5511"
-      }, 
+      },
       {
-         "affiliation": "Netherlands eScience Center", 
-         "name": "Spaaks, Jurriaan H.", 
+         "affiliation": "Netherlands eScience Center",
+         "name": "Spaaks, Jurriaan H.",
          "orcid": "0000-0002-7064-4069"
-      }, 
+      },
       {
-         "affiliation": "Netherlands eScience Center", 
-         "name": "Diblen, Faruk", 
+         "affiliation": "Netherlands eScience Center",
+         "name": "Diblen, Faruk",
          "orcid": "0000-0002-0989-929X"
-      }, 
+      },
       {
-         "affiliation": "Netherlands eScience Center", 
-         "name": "Verhoeven, Stefan", 
+         "affiliation": "Netherlands eScience Center",
+         "name": "Verhoeven, Stefan",
          "orcid": "0000-0002-5821-2060"
-      }, 
+      },
       {
-         "affiliation": "Netherlands eScience Center", 
-         "name": "Geng, Cunliang", 
+         "affiliation": "Netherlands eScience Center",
+         "name": "Geng, Cunliang",
          "orcid": "0000-0002-1409-8358"
-      }, 
+      },
       {
-         "affiliation": "Netherlands eScience Center", 
-         "name": "Meijer, Christiaan", 
+         "affiliation": "Netherlands eScience Center",
+         "name": "Meijer, Christiaan",
          "orcid": "0000-0002-5529-5761"
-      }, 
+      },
       {
-         "affiliation": "University of Glasgow", 
-         "name": "Rogers, Simon", 
+         "affiliation": "University of Glasgow",
+         "name": "Rogers, Simon",
          "orcid": "0000-0003-3578-4477"
-      }, 
+      },
       {
-         "affiliation": "Netherlands eScience Center", 
-         "name": "Belloum, Adam", 
+         "affiliation": "Netherlands eScience Center",
+         "name": "Belloum, Adam",
          "orcid": "0000-0001-6306-6937"
-      }, 
+      },
       {
-         "affiliation": "Netherlands eScience Center", 
-         "name": "Spreeuw, Hanno", 
+         "affiliation": "Netherlands eScience Center",
+         "name": "Spreeuw, Hanno",
          "orcid": "0000-0002-5057-0322"
       },
       {
-         "affiliation": "University of Glasgow", 
-         "name": "Villanueva Castilla, Efrain Manuel", 
+         "affiliation": "University of Glasgow",
+         "name": "Villanueva Castilla, Efrain Manuel",
          "orcid": "0000-0001-7665-3575"
       },
       {
-         "affiliation": "University of Glasgow", 
-         "name": "Ashouritaklimi, Kianoosh", 
+         "affiliation": "University of Glasgow",
+         "name": "Ashouritaklimi, Kianoosh",
          "orcid": "0000-0001-6317-3193"
+      },
+      {
+         "affiliation": "Netherlands eScience Center",
+         "name": "de Jonge, Niek",
+         "orcid": "0000-0002-3054-6210"
       }
-   ], 
-   "description": "Python library for fuzzy comparison of mass spectrum data and other Python objects", 
+   ],
+   "description": "Python library for fuzzy comparison of mass spectrum data and other Python objects",
    "keywords": [
-      "python", 
-      "similarity measures", 
-      "mass spectrometry", 
-      "fuzzy matching", 
+      "python",
+      "similarity measures",
+      "mass spectrometry",
+      "fuzzy matching",
       "fuzzy search"
-   ], 
+   ],
    "license": {
       "id": "Apache-2.0"
-   }, 
+   },
    "title": "matchms"
 }
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a1dd5a46..0ee95919 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- Added filter function 'require_precursor_mz' and added 1 assert function in 'ModifiedCosine' [#191](https://github.com/matchms/matchms/pull/191)
+
 - `make_charge_int()` to convert charge field to integer [#184](https://github.com/matchms/matchms/issues/184)
 
 ### Changed
diff --git a/CITATION.cff b/CITATION.cff
index 7e28efcb..8559f7db 100644
--- a/CITATION.cff
+++ b/CITATION.cff
@@ -63,6 +63,11 @@ authors:
     family-names: Ashouritaklimi
     given-names: Kianoosh
     orcid: "https://orcid.org/0000-0001-6317-3193"
+  -
+    affiliation: "Netherlands eScience Center"
+    family-names: de Jonge
+    given-names: Niek
+    orcid: "https://orcid.org/0000-0002-3054-6210"
 
 cff-version: "1.1.0"
 keywords:
diff --git a/matchms/filtering/require_precursor_mz.py b/matchms/filtering/require_precursor_mz.py
new file mode 100644
index 00000000..fc95cb16
--- /dev/null
+++ b/matchms/filtering/require_precursor_mz.py
@@ -0,0 +1,34 @@
+from typing import Union
+from ..typing import SpectrumType
+
+
+def require_precursor_mz(spectrum_in: SpectrumType
+                         ) -> Union[SpectrumType, None]:
+
+    """Returns None if there is no precursor_mz or if <=0
+
+    Parameters
+    ----------
+    spectrum_in:
+        Input spectrum.
+    """
+    if spectrum_in is None:
+        return None
+
+    spectrum = spectrum_in.clone()
+
+    precursor_mz = spectrum.get("precursor_mz", None)
+    if precursor_mz is None:
+        pepmass = spectrum.get("pepmass", None)
+        assert pepmass is None or not isinstance(pepmass[0], (float, int)), \
+            "Found 'pepmass' but no 'precursor_mz'. " \
+            "Consider applying 'add_precursor_mz' filter first."
+        return None
+
+    assert isinstance(precursor_mz, (float, int)), \
+        ("Expected 'precursor_mz' to be a scalar number.",
+         "Consider applying 'add_precursor_mz' filter first.")
+    if precursor_mz <= 0:
+        return None
+
+    return spectrum
diff --git a/matchms/similarity/ModifiedCosine.py b/matchms/similarity/ModifiedCosine.py
index 40b4bbf6..2aee5e97 100644
--- a/matchms/similarity/ModifiedCosine.py
+++ b/matchms/similarity/ModifiedCosine.py
@@ -90,8 +90,15 @@ class ModifiedCosine(BaseSimilarity):
             zero_pairs = collect_peak_pairs(spec1, spec2, self.tolerance, shift=0.0,
                                             mz_power=self.mz_power,
                                             intensity_power=self.intensity_power)
-            message = "Precursor_mz missing. Apply 'add_precursor_mz' filter first."
-            assert reference.get("precursor_mz") and query.get("precursor_mz"), message
+            message_precursor_missing = \
+                "Precursor_mz missing. Apply 'add_precursor_mz' filter first."
+            assert reference.get("precursor_mz") \
+                   and query.get("precursor_mz"), message_precursor_missing
+            message_precursor_below_0 = "Expect precursor to be positive number." \
+                                        "Apply 'require_precursor_mz' first"
+            assert reference.get("precursor_mz") > 0 \
+                   and query.get("precursor_mz") > 0, message_precursor_below_0
+
             mass_shift = reference.get("precursor_mz") - query.get("precursor_mz")
             nonzero_pairs = collect_peak_pairs(spec1, spec2, self.tolerance, shift=mass_shift,
                                                mz_power=self.mz_power,


Test patch:
diff --git a/tests/test_require_precursor_mz.py b/tests/test_require_precursor_mz.py
new file mode 100644
index 00000000..2ebe3d0f
--- /dev/null
+++ b/tests/test_require_precursor_mz.py
@@ -0,0 +1,86 @@
+import numpy
+import pytest
+from matchms import Spectrum
+from matchms.filtering.require_precursor_mz import require_precursor_mz
+
+
+def test_require_precursor_mz_pass():
+    """Test with correct precursor mz present."""
+    mz = numpy.array([10, 20, 30, 40], dtype="float")
+    intensities = numpy.array([0, 1, 10, 100], dtype="float")
+    spectrum_in = Spectrum(mz=mz, intensities=intensities)
+    spectrum_in.set("precursor_mz", 60.)
+
+    spectrum = require_precursor_mz(spectrum_in)
+
+    assert spectrum == spectrum_in, "Expected no changes."
+
+
+def test_require_precursor_mz_fail_because_zero():
+    """Test if spectrum is None when precursor_mz == 0"""
+    mz = numpy.array([10, 20, 30, 40], dtype="float")
+    intensities = numpy.array([0, 1, 10, 100], dtype="float")
+    spectrum_in = Spectrum(mz=mz, intensities=intensities)
+    spectrum_in.set("precursor_mz", 0.0)
+
+    spectrum = require_precursor_mz(spectrum_in)
+
+    assert spectrum is None, "Expected spectrum to be None."
+
+
+def test_require_precursor_mz_fail_because_below_zero():
+    """Test if spectrum is None when precursor_mz < 0"""
+    mz = numpy.array([10, 20, 30, 40], dtype="float")
+    intensities = numpy.array([0, 1, 10, 100], dtype="float")
+    spectrum_in = Spectrum(mz=mz, intensities=intensities)
+    spectrum_in.set("precursor_mz", -3.5)
+
+    spectrum = require_precursor_mz(spectrum_in)
+
+    assert spectrum is None, "Expected spectrum to be None."
+
+
+def test_if_spectrum_is_cloned():
+    """Test if filter is correctly cloning the input spectrum."""
+    mz = numpy.array([10, 20, 30, 40], dtype="float")
+    intensities = numpy.array([0, 1, 10, 100], dtype="float")
+    spectrum_in = Spectrum(mz=mz, intensities=intensities)
+    spectrum_in.set("precursor_mz", 1.)
+
+    spectrum = require_precursor_mz(spectrum_in)
+    spectrum.set("testfield", "test")
+
+    assert not spectrum_in.get("testfield"), \
+        "Expected input spectrum to remain unchanged."
+
+
+def test_require_precursor_mz_without_precursor_mz():
+    """Test if None is returned for missing precursor-mz."""
+    spectrum_in = Spectrum(mz=numpy.array([10, 20, 30, 40], dtype="float"),
+                           intensities=numpy.array([0, 1, 10, 100],
+                                                   dtype="float"),
+                           metadata={})
+
+    spectrum = require_precursor_mz(spectrum_in)
+
+    assert spectrum is None, "Expected spectrum to be None."
+
+
+def test_require_precursor_mz_with_wrong_precursor_mz():
+    """Test if correct assert error is raised for precursor-mz as string."""
+    spectrum_in = Spectrum(mz=numpy.array([10, 20, 30, 40], dtype="float"),
+                           intensities=numpy.array([0, 1, 10, 100],
+                                                   dtype="float"),
+                           metadata={"precursor_mz": "445.0"})
+
+    with pytest.raises(AssertionError) as msg:
+        _ = require_precursor_mz(spectrum_in)
+
+    assert "Expected 'precursor_mz' to be a scalar number." in str(msg.value)
+
+
+def test_require_precursor_mz_with_input_none():
+    """Test if input spectrum is None."""
+    spectrum_in = None
+    spectrum = require_precursor_mz(spectrum_in)
+    assert spectrum is None