Repository: matchms/matchms
Pull request #183 (2021-02-25 09:11:52)

Fix string charges
Based on PR from fork, done by @davidwhealey
Original message:

Some json spectra contain string-formatted charges. This pull request adds a filter for string-formatted charges to filtering.make_charge_scalar

- For more detailed discussion I opened #184.
- I added a unit test
- Give rights to @davidwhealey
- Switch to `make_charge_int()`, now marked as deprecated: `make_charge_scalar()`

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



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index bd59a321..a1dd5a46 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- `make_charge_int()` to convert charge field to integer [#184](https://github.com/matchms/matchms/issues/184)
+
+### Changed
+
+- now deprecated: `make_charge_scalar()`, use `make_charge_int()` instead [#183](https://github.com/matchms/matchms/pull/183)
+
 ### Fixed
 
 - Very minor bugs in `add_parent_mass` [#188](https://github.com/matchms/matchms/pull/188)
diff --git a/matchms/filtering/__init__.py b/matchms/filtering/__init__.py
index c57911f3..249386d1 100644
--- a/matchms/filtering/__init__.py
+++ b/matchms/filtering/__init__.py
@@ -53,6 +53,7 @@ from .derive_smiles_from_inchi import derive_smiles_from_inchi
 from .harmonize_undefined_inchi import harmonize_undefined_inchi
 from .harmonize_undefined_inchikey import harmonize_undefined_inchikey
 from .harmonize_undefined_smiles import harmonize_undefined_smiles
+from .make_charge_int import make_charge_int
 from .make_charge_scalar import make_charge_scalar
 from .make_ionmode_lowercase import make_ionmode_lowercase
 from .normalize_intensities import normalize_intensities
@@ -88,6 +89,7 @@ __all__ = [
     "harmonize_undefined_inchi",
     "harmonize_undefined_inchikey",
     "harmonize_undefined_smiles",
+    "make_charge_int",
     "make_charge_scalar",
     "make_ionmode_lowercase",
     "normalize_intensities",
diff --git a/matchms/filtering/default_filters.py b/matchms/filtering/default_filters.py
index 786fbd94..8bbd4303 100644
--- a/matchms/filtering/default_filters.py
+++ b/matchms/filtering/default_filters.py
@@ -6,7 +6,7 @@ from .correct_charge import correct_charge
 from .derive_adduct_from_name import derive_adduct_from_name
 from .derive_formula_from_name import derive_formula_from_name
 from .derive_ionmode import derive_ionmode
-from .make_charge_scalar import make_charge_scalar
+from .make_charge_int import make_charge_int
 from .make_ionmode_lowercase import make_ionmode_lowercase
 from .set_ionmode_na_when_missing import set_ionmode_na_when_missing
 
@@ -17,7 +17,7 @@ def default_filters(spectrum: SpectrumType) -> SpectrumType:
 
     Collection is
 
-    1. :meth:`~matchms.filtering.make_charge_scalar`
+    1. :meth:`~matchms.filtering.make_charge_int`
     2. :meth:`~matchms.filtering.make_ionmode_lowercase`
     3. :meth:`~matchms.filtering.set_ionmode_na_when_missing`
     4. :meth:`~matchms.filtering.add_compound_name`
@@ -29,7 +29,7 @@ def default_filters(spectrum: SpectrumType) -> SpectrumType:
     10. :meth:`~matchms.filtering.correct_charge`
 
     """
-    spectrum = make_charge_scalar(spectrum)
+    spectrum = make_charge_int(spectrum)
     spectrum = make_ionmode_lowercase(spectrum)
     spectrum = set_ionmode_na_when_missing(spectrum)
     spectrum = add_compound_name(spectrum)
diff --git a/matchms/filtering/make_charge_int.py b/matchms/filtering/make_charge_int.py
new file mode 100644
index 00000000..fa305e53
--- /dev/null
+++ b/matchms/filtering/make_charge_int.py
@@ -0,0 +1,23 @@
+from ..typing import SpectrumType
+
+
+def make_charge_int(spectrum_in: SpectrumType) -> SpectrumType:
+    """Convert charge field to integer (if possible)."""
+    if spectrum_in is None:
+        return None
+
+    spectrum = spectrum_in.clone()
+
+    # Avoid pyteomics ChargeList
+    if isinstance(spectrum.get("charge", None), list):
+        spectrum.set("charge", int(spectrum.get("charge")[0]))
+
+    # convert string charges to int
+    if isinstance(spectrum.get("charge", None), str):
+        try:
+            charge_int = int(spectrum.get('charge'))
+            spectrum.set("charge", charge_int)
+        except ValueError:
+            print(f"Found charge ({spectrum.get('charge')}) cannot be converted to integer.")
+
+    return spectrum
diff --git a/matchms/filtering/make_charge_scalar.py b/matchms/filtering/make_charge_scalar.py
index d1239ce8..15419236 100644
--- a/matchms/filtering/make_charge_scalar.py
+++ b/matchms/filtering/make_charge_scalar.py
@@ -1,15 +1,13 @@
+from deprecated.sphinx import deprecated
 from ..typing import SpectrumType
+from .make_charge_int import make_charge_int
 
 
+@deprecated(version='0.8.2', reason="Use expanded make_charge_int() instead.")
 def make_charge_scalar(spectrum_in: SpectrumType) -> SpectrumType:
-    """Convert charge field to scalar (if necessary)."""
-    if spectrum_in is None:
-        return None
+    """Convert charge field to scalar (if necessary).
 
-    spectrum = spectrum_in.clone()
+    Deprecated function, now replaced by make_charge_int().
+    """
 
-    # Avoid pyteomics ChargeList
-    if isinstance(spectrum.get("charge", None), list):
-        spectrum.set("charge", int(spectrum.get("charge")[0]))
-
-    return spectrum
+    return make_charge_int(spectrum_in)


Test patch:
diff --git a/tests/test_make_charge_int.py b/tests/test_make_charge_int.py
new file mode 100644
index 00000000..372e3fe1
--- /dev/null
+++ b/tests/test_make_charge_int.py
@@ -0,0 +1,29 @@
+import numpy
+import pytest
+from matchms import Spectrum
+from matchms.filtering import make_charge_int
+
+
+@pytest.mark.parametrize("input_charge, corrected_charge", [('+1', 1),
+                                                            ('1', 1),
+                                                            (' 1 ', 1),
+                                                            ('-2', -2),
+                                                            ([-1, "stuff"], -1),
+                                                            (['-3'], -3),
+                                                            ('0', 0),
+                                                            ('n/a', 'n/a')])
+def test_make_charge_int(input_charge, corrected_charge):
+    """Test if example inputs are correctly converted"""
+    spectrum_in = Spectrum(mz=numpy.array([100, 200.]),
+                           intensities=numpy.array([0.7, 0.1]),
+                           metadata={'charge': input_charge})
+
+    spectrum = make_charge_int(spectrum_in)
+    assert(spectrum.get("charge") == corrected_charge), "Expected different charge integer"
+
+
+def test_empty_spectrum():
+    spectrum_in = None
+    spectrum = make_charge_int(spectrum_in)
+
+    assert spectrum is None, "Expected different handling of None spectrum."
diff --git a/tests/test_make_charge_scalar.py b/tests/test_make_charge_scalar.py
new file mode 100644
index 00000000..4d165328
--- /dev/null
+++ b/tests/test_make_charge_scalar.py
@@ -0,0 +1,26 @@
+import numpy
+import pytest
+from matchms import Spectrum
+from matchms.filtering import make_charge_scalar
+
+
+@pytest.mark.parametrize("input_charge, corrected_charge", [('+1', 1),
+                                                            ('1', 1),
+                                                            (' 1 ', 1),
+                                                            ('-1', -1),
+                                                            ([-1, "stuff"], -1)])
+def test_make_charge_scalar(input_charge, corrected_charge):
+    """Test if example inputs are correctly converted"""
+    spectrum_in = Spectrum(mz=numpy.array([100, 200.]),
+                           intensities=numpy.array([0.7, 0.1]),
+                           metadata={'charge': input_charge})
+
+    spectrum = make_charge_scalar(spectrum_in)
+    assert(spectrum.get("charge") == corrected_charge), "Expected different charge integer"
+
+
+def test_empty_spectrum():
+    spectrum_in = None
+    spectrum = make_charge_scalar(spectrum_in)
+
+    assert spectrum is None, "Expected different handling of None spectrum."