Repository: matchms/matchms
Pull request #176 (2021-02-12 15:28:04)

Fix issue with n_max in filter
Addresses #177 

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=176&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=176&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=176&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=176&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=176&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=176&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=176&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=176&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=176&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=176&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=matchms_matchms&pullRequest=176&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=176&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=176&metric=duplicated_lines_density&view=list) No Duplication information


jspaaks: Superseded by https://github.com/matchms/matchms/pull/180, closing this one

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2dcfb9d6..4e241dc6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## Fixed
 
 - too unspecific formula detection (and removal) from given compound names in `derive_formula_from_name` [#172](https://github.com/matchms/matchms/issues/172)
+- no longer ignore n_max setting in `reduce_to_number_of_peaks` filter [#177](https://github.com/matchms/matchms/issues/177)
 
 ## [0.7.0] - 2021-01-04
 
diff --git a/matchms/filtering/reduce_to_number_of_peaks.py b/matchms/filtering/reduce_to_number_of_peaks.py
index f93e5870..e6ce9c9c 100644
--- a/matchms/filtering/reduce_to_number_of_peaks.py
+++ b/matchms/filtering/reduce_to_number_of_peaks.py
@@ -1,10 +1,11 @@
 from math import ceil
 from typing import Optional
+import numpy
 from ..Spikes import Spikes
 from ..typing import SpectrumType
 
 
-def reduce_to_number_of_peaks(spectrum_in: SpectrumType, n_required: int = 1, n_max: int = 100,
+def reduce_to_number_of_peaks(spectrum_in: SpectrumType, n_required: int = 1, n_max: int = numpy.inf,
                               ratio_desired: Optional[float] = None) -> SpectrumType:
     """Lowest intensity peaks will be removed when it has more peaks than desired.
 
@@ -16,7 +17,7 @@ def reduce_to_number_of_peaks(spectrum_in: SpectrumType, n_required: int = 1, n_
         Number of minimum required peaks. Spectra with fewer peaks will be set
         to 'None'. Default is 1.
     n_max:
-        Maximum number of peaks. Remove peaks if more peaks are found.
+        Maximum number of peaks. Remove peaks if more peaks are found. Default is inf.
     ratio_desired:
         Set desired ratio between maximum number of peaks and parent mass.
         For spectra without parent mass (e.g. GCMS spectra) this will raise an
@@ -27,7 +28,7 @@ def reduce_to_number_of_peaks(spectrum_in: SpectrumType, n_required: int = 1, n_
         parent_mass = spectrum.get("parent_mass", None)
         if parent_mass and ratio_desired:
             n_desired_by_mass = int(ceil(ratio_desired * parent_mass))
-            return max(n_required, n_desired_by_mass)
+            return min(max(n_required, n_desired_by_mass), n_max)
         if not ratio_desired:
             return n_max
         raise ValueError("Cannot use ratio_desired for spectrum without parent_mass.")


Test patch:
diff --git a/tests/test_reduce_to_number_of_peaks.py b/tests/test_reduce_to_number_of_peaks.py
index 593992d3..7baf01d0 100644
--- a/tests/test_reduce_to_number_of_peaks.py
+++ b/tests/test_reduce_to_number_of_peaks.py
@@ -65,8 +65,8 @@ def test_reduce_to_number_of_peaks_required_2_desired_2():
     assert spectrum.peaks.mz.tolist() == [30., 40.], "Expected different peaks to remain."
 
 
-def test_reduce_to_number_of_peaks_required_2_desired_3():
-    """Here: ratio_desired * parent_mass is 3, more than n_required."""
+def test_reduce_to_number_of_peaks_required_3_desired_2():
+    """Here: ratio_desired * parent_mass is 2, less than n_required."""
     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,
@@ -78,6 +78,19 @@ def test_reduce_to_number_of_peaks_required_2_desired_3():
     assert spectrum.peaks.mz.tolist() == [20., 30., 40.], "Expected different peaks to remain."
 
 
+def test_reduce_to_number_of_peaks_required_2_desired_6_max_4():
+    """Here: ratio_desired * parent_mass is 6, more than n_required and more than n_max."""
+    mz = numpy.array([10, 20, 30, 40, 50, 60], dtype="float")
+    intensities = numpy.array([1, 1, 10, 100, 50, 20], dtype="float")
+    spectrum_in = Spectrum(mz=mz, intensities=intensities,
+                           metadata={"parent_mass": 60})
+
+    spectrum = reduce_to_number_of_peaks(spectrum_in, n_required=3, n_max=4, ratio_desired=0.1)
+
+    assert len(spectrum.peaks) == 4, "Expected that only 4 peaks remain."
+    assert spectrum.peaks.mz.tolist() == [30., 40., 50., 60.], "Expected different peaks to remain."
+
+
 def test_reduce_to_number_of_peaks_desired_5_check_sorting():
     """Check if mz and intensities order is sorted correctly """
     mz = numpy.array([10, 20, 30, 40, 50, 60], dtype="float")