Repository: matchms/matchms-backup
Issue category: Security Vulnerability

Issue:
openbabel has contagious license
https://tldrlegal.com/license/gnu-general-public-license-v2


Hints:
Openbabel seems like a lot of pain. So far I didn't get the time to look at it enough. But I hope that I can soon try to look for alternatives. There are certainly several potential alternatives, but we would have to test them on a large number of conversions to find if they have a similar or better success rate.

Some alternatives I want to look into are:
- **rdkit** (also not a simple library, but we use it already anyway. However, I remember that in the past I got less correct conversions).
- **openeye** 
- **ChemSpider**, see e.g. https://www.inchi-trust.org/inchi-post/chemistry-programming-with-python-convert-a-smile-string-to-inchi-using-chemspider-tutorial/
- **PubChemPy**
I ran rdkit and openbabel on about 45000 inchi's from the actual dataset. Unfortunately I got a similar result than a year ago with rdkit retrieving far fewer smiles from the given inchi.
 
```python
from rdkit import Chem

smiles = Chem.MolToSmiles(Chem.MolFromInchi(inchi))
```

Patch:
diff --git a/environment.yml b/environment.yml
index b38d68a..f46d4a2 100644
--- a/environment.yml
+++ b/environment.yml
@@ -7,7 +7,6 @@ dependencies:
   - gensim==3.8.0
   - matplotlib==3.2.1
   - numpy==1.18.1
-  - openbabel==3.0.0
   - pip==20.0.2
   - pyteomics==4.2
   - python==3.7
diff --git a/matchms/filtering/derive_inchi_from_smiles.py b/matchms/filtering/derive_inchi_from_smiles.py
index daafe86..c972014 100644
--- a/matchms/filtering/derive_inchi_from_smiles.py
+++ b/matchms/filtering/derive_inchi_from_smiles.py
@@ -1,5 +1,5 @@
 from ..typing import SpectrumType
-from ..utils import mol_converter, is_valid_inchi, is_valid_smiles
+from ..utils import convert_smiles_to_inchi, is_valid_inchi, is_valid_smiles
 
 
 def derive_inchi_from_smiles(spectrum_in: SpectrumType) -> SpectrumType:
@@ -12,12 +12,9 @@ def derive_inchi_from_smiles(spectrum_in: SpectrumType) -> SpectrumType:
     smiles = spectrum.get("smiles")
 
     if not is_valid_inchi(inchi) and is_valid_smiles(smiles):
-        inchi = mol_converter(smiles, "smi", "inchi")
-        if not inchi:
-            # Second try: use smiley ("smy") parser
-            inchi = mol_converter(smiles, "smy", "inchi")
+        inchi = convert_smiles_to_inchi(smiles)
         if inchi:
-            inchi = inchi.replace('\n', '').replace('\t', '').replace('\r', '')
+            inchi = inchi.rstrip()
             spectrum.set("inchi", inchi)
         else:
             print("Could not convert smiles", smiles, "to InChI.")
diff --git a/matchms/filtering/derive_inchikey_from_inchi.py b/matchms/filtering/derive_inchikey_from_inchi.py
index 0a9c0df..32d6b91 100644
--- a/matchms/filtering/derive_inchikey_from_inchi.py
+++ b/matchms/filtering/derive_inchikey_from_inchi.py
@@ -1,5 +1,5 @@
-from ..utils import mol_converter, is_valid_inchi, is_valid_inchikey
 from ..typing import SpectrumType
+from ..utils import convert_inchi_to_inchikey, is_valid_inchi, is_valid_inchikey
 
 
 def derive_inchikey_from_inchi(spectrum_in: SpectrumType) -> SpectrumType:
@@ -13,7 +13,7 @@ def derive_inchikey_from_inchi(spectrum_in: SpectrumType) -> SpectrumType:
     inchikey = spectrum.get("inchikey")
 
     if is_valid_inchi(inchi) and not is_valid_inchikey(inchikey):
-        inchikey = mol_converter(inchi, "inchi", "inchikey")
+        inchikey = convert_inchi_to_inchikey(inchi)
         if inchikey:
             spectrum.set("inchikey", inchikey)
         else:
diff --git a/matchms/filtering/derive_smiles_from_inchi.py b/matchms/filtering/derive_smiles_from_inchi.py
index 1cac9e0..b23ea94 100644
--- a/matchms/filtering/derive_smiles_from_inchi.py
+++ b/matchms/filtering/derive_smiles_from_inchi.py
@@ -1,5 +1,5 @@
-from ..utils import mol_converter, is_valid_smiles, is_valid_inchi
 from ..typing import SpectrumType
+from ..utils import convert_inchi_to_smiles, is_valid_smiles, is_valid_inchi
 
 
 def derive_smiles_from_inchi(spectrum_in: SpectrumType) -> SpectrumType:
@@ -12,9 +12,9 @@ def derive_smiles_from_inchi(spectrum_in: SpectrumType) -> SpectrumType:
     smiles = spectrum.get("smiles")
 
     if not is_valid_smiles(smiles) and is_valid_inchi(inchi):
-        smiles = mol_converter(inchi, "inchi", "smi")
+        smiles = convert_inchi_to_smiles(inchi)
         if smiles:
-            smiles = smiles.replace('\n', '').replace('\t', '').replace('\r', '')
+            smiles = smiles.rstrip()
             spectrum.set("smiles", smiles)
         else:
             print("Could not convert InChI", inchi, "to smiles.")
diff --git a/matchms/utils.py b/matchms/utils.py
index 02bd690..c204bbb 100644
--- a/matchms/utils.py
+++ b/matchms/utils.py
@@ -1,32 +1,47 @@
 import re
-from openbabel import openbabel as ob
 from rdkit import Chem
 
 
+def convert_smiles_to_inchi(smiles):
+    return mol_converter(smiles, "smiles", "inchi")
+
+
+def convert_inchi_to_smiles(inchi):
+    return mol_converter(inchi, "inchi", "smiles")
+
+
+def convert_inchi_to_inchikey(inchi):
+    return mol_converter(inchi, "inchi", "inchikey")
+
+
 def mol_converter(mol_input, input_type, output_type):
-    """Convert molecular representations using openbabel.
+    """Convert molecular representations using rdkit.
 
-    Convert for instance from smiles to inchi or inchi to inchikey.
+    Convert for from smiles or inchi to inchi, smiles, or inchikey.
 
     Args:
     ----
     mol_input: str
-        Input data, e.g. inchi or smiles.
+        Input data, inchi or smiles.
     input_type: str
-        Define input type (as named in openbabel). E.g. "smi"for smiles and "inchi" for inchi.
+        Define input type: "smiles" for smiles and "inchi" for inchi.
     output_type: str
-        Define input type (as named in openbabel). E.g. "smi"for smiles and "inchi" for inchi.
+        Define output type: "smiles", "inchi", or "inchikey".
     """
-    conv = ob.OBConversion()
-    conv.SetInAndOutFormats(input_type, output_type)
-    mol = ob.OBMol()
-    if conv.ReadString(mol, mol_input):
-        mol_output = conv.WriteString(mol)
-    else:
-        print("Error when converting", mol_input)
-        mol_output = None
+    input_function = {"inchi": Chem.MolFromInchi,
+                      "smiles": Chem.MolFromSmiles}
+    output_function = {"inchi": Chem.MolToInchi,
+                       "smiles": Chem.MolToSmiles,
+                       "inchikey": Chem.MolToInchiKey}
+
+    mol = input_function[input_type](mol_input.strip('"'))
+    if mol is None:
+        return None
 
-    return mol_output
+    output = output_function[output_type](mol)
+    if output:
+        return output
+    return None
 
 
 def is_valid_inchi(inchi):
@@ -42,7 +57,7 @@ def is_valid_inchi(inchi):
     # First quick test to avoid excess in-depth testing
     if inchi is None:
         return False
-    inchi = inchi.replace('"', "")
+    inchi = inchi.strip('"')
     regexp = r"(InChI=1|1)(S\/|\/)[0-9, A-Z, a-z,\.]{2,}\/(c|h)[0-9]"
     if not re.search(regexp, inchi):
         return False
diff --git a/meta.yaml b/meta.yaml
index f134d0e..495b22c 100644
--- a/meta.yaml
+++ b/meta.yaml
@@ -36,7 +36,6 @@ requirements:
     - python==3.7
     - matplotlib==3.2.1
     - numpy==1.18.1
-    - openbabel==3.0.0
     - pyyaml==5.3.1
     - pycodestyle==2.5.0
     - pytest==5.4.1


Test patch:
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 1918816..7816846 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -1,18 +1,81 @@
-from matchms.utils import is_valid_inchikey
+from matchms.utils import mol_converter
+from matchms.utils import is_valid_inchi, is_valid_inchikey, is_valid_smiles
+
+
+def test_mol_converter_smiles_to_inchi():
+    """Test if smiles is correctly converted to inchi."""
+    mol_input = "C[Si](Cn1cncn1)(c1ccc(F)cc1)"
+    output_inchi = mol_converter(mol_input, "smiles", "inchi")
+    assert output_inchi == "InChI=1S/C10H11FN3Si/c1-15(8-14-7-12-6-13-14)10-4-2-9(11)3-5-10/h2-7H,8H2,1H3"
+
+
+def test_mol_converter_inchi_to_smiles():
+    """Test if inchi is correctly converted to smiles."""
+    mol_input = "InChI=1S/C10H11FN3Si/c1-15(8-14-7-12-6-13-14)10-4-2-9(11)3-5-10/h2-7H,8H2,1H3"
+    output_smiles = mol_converter(mol_input, "inchi", "smiles")
+    assert output_smiles == "C[Si](Cn1cncn1)=C1C=C[C](F)C=C1"
+
+
+def test_mol_converter_smiles_to_inchikey():
+    """Test if smiles is correctly converted to inchikey."""
+    mol_input = "C[Si](Cn1cncn1)(c1ccc(F)cc1)"
+    output_inchikey = mol_converter(mol_input, "smiles", "inchikey")
+    assert output_inchikey == "HULABQRTZQYJBQ-UHFFFAOYSA-N"
 
 
 def test_is_valid_inchikey():
     """Test if strings are correctly classified."""
     inchikeys_true = ["XYLJNLCSTIOKRM-UHFFFAOYSA-N"]
-    inchikeys_false = ["XYLJNLCSTIOKRM-UHFFFAOYSA",
-                       "XYLJNLCSTIOKRMRUHFFFAOYSASN",
-                       "XYLJNLCSTIOKR-MUHFFFAOYSA-N",
-                       "XYLJNLCSTIOKRM-UHFFFAOYSA-NN",
-                       "Brcc(NC2=NCN2)-ccc3nccnc1-3",
-                       "2YLJNLCSTIOKRM-UHFFFAOYSA-N",
-                       "XYLJNLCSTIOKRM-aaaaaaaaaa-a"]
+    inchikeys_false = [
+        "XYLJNLCSTIOKRM-UHFFFAOYSA",
+        "XYLJNLCSTIOKRMRUHFFFAOYSASN",
+        "XYLJNLCSTIOKR-MUHFFFAOYSA-N",
+        "XYLJNLCSTIOKRM-UHFFFAOYSA-NN",
+        "Brcc(NC2=NCN2)-ccc3nccnc1-3",
+        "2YLJNLCSTIOKRM-UHFFFAOYSA-N",
+        "XYLJNLCSTIOKRM-aaaaaaaaaa-a"
+    ]
 
     for inchikey in inchikeys_true:
         assert is_valid_inchikey(inchikey), "Expected inchikey is True."
     for inchikey in inchikeys_false:
         assert not is_valid_inchikey(inchikey), "Expected inchikey is False."
+
+
+def test_is_valid_inchi():
+    """Test if strings are correctly classified."""
+    inchi_true = [
+        "InChI=1S/C2H7N3/c1-5-2(3)4/h1H3,(H4,3,4,5)",
+        '"InChI=1S/C2H7N3/c1-5-2(3)4/h1H3,(H4,3,4,5)"'
+    ]
+    inchi_false = [
+        "1S/C2H7N3/c1-5-2(3)4/h1H3,(H4,3,4,5)",
+        "InChI=1S/C2H7N3/c152(3)4/h1H3,(H4,3,4,5)",
+        "InChI=C2H7N3/c1-5-2(3)4/h1H3,(H4,3,4,5)",
+        "InChI=1S/C2H7N3/c1-5-2(3)"
+    ]
+
+    for inchi in inchi_true:
+        assert is_valid_inchi(inchi), "Expected inchi is True."
+    for inchi in inchi_false:
+        assert not is_valid_inchi(inchi), "Expected inchi is False."
+
+
+def test_is_valid_smiles():
+    """Test if strings are correctly classified."""
+    smiles_true = [
+        r"CN1COCN(CC2=CN=C(Cl)S2)\C1=N\[N+]([O-])=O",
+        r"CN1N(C(=O)C=C1C)c1ccccc1",
+        r"COC(=O)C1=CN=CC=N1"
+    ]
+    smiles_false = [
+        r"CN1N(C(=O)C=C1C)c1cccccx1",
+        r"CN1COCN(CC2=CN=C(Cl)S2)\C1=N\[N+++]([O-])=O",
+        r"COC(=O[)]C1=CN=CC=N1",
+        r"1S/C2H7N3/c1-5-2(3)4"
+    ]
+
+    for smiles in smiles_true:
+        assert is_valid_smiles(smiles), "Expected smiles is True."
+    for smiles in smiles_false:
+        assert not is_valid_smiles(smiles), "Expected smiles is False."