Repository: matchms/matchms (python)

Issue:
Add retention data filters
As discussed in #251 , adding RI and RT filters to clean the metadata in the spectrum object would be good.

One additional idea would be for the retention time filter to allow passing a conversion factor which converts the RT to seconds and use this as a canonical unit for RT?


Pull request description:
Implemented filters to add retentiontion time & index
Implemented filters for retention time and retention index to add them to the metadata.

The current versions don't perform any dedicated error handling or strictness as this should be implemented in a harmonized way across all filters, so it can be added on later, see #263 .

Closes #260 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 95f5f529..3509bf47 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,8 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - `Spectrum()` objects now also allows generating hashes, e.g. `hash(spectrum)` [#259](https://github.com/matchms/matchms/pull/259)
 - `Spectrum()` objects can generate `.spectrum_hash()` and `.metadata_hash()` to track changes to peaks or metadata [#259](https://github.com/matchms/matchms/pull/259)
-- 'load_from_mf()` now accepts both a path to a mgf file or a file-like object from a preloaded MGF file [#258](https://github.com/matchms/matchms/pull/258)
-
+- `load_from_mgf()` now accepts both a path to a mgf file or a file-like object from a preloaded MGF file [#258](https://github.com/matchms/matchms/pull/258)
+- Added `add_retention` filters with function `add_retention_time()` and `add_retention_index()` [#265](https://github.com/matchms/matchms/pull/265)
 ### Changed
 
 - Code linting triggered by pylint update [#257](https://github.com/matchms/matchms/pull/257)
diff --git a/matchms/filtering/__init__.py b/matchms/filtering/__init__.py
index 249386d1..93ccdaa9 100644
--- a/matchms/filtering/__init__.py
+++ b/matchms/filtering/__init__.py
@@ -41,6 +41,8 @@ from .add_fingerprint import add_fingerprint
 from .add_losses import add_losses
 from .add_parent_mass import add_parent_mass
 from .add_precursor_mz import add_precursor_mz
+from .add_retention import add_retention_index
+from .add_retention import add_retention_time
 from .clean_compound_name import clean_compound_name
 from .correct_charge import correct_charge
 from .default_filters import default_filters
@@ -77,6 +79,8 @@ __all__ = [
     "add_losses",
     "add_parent_mass",
     "add_precursor_mz",
+    "add_retention_index",
+    "add_retention_time",
     "clean_compound_name",
     "correct_charge",
     "default_filters",
diff --git a/matchms/filtering/add_precursor_mz.py b/matchms/filtering/add_precursor_mz.py
index 2022653f..6aeb8479 100644
--- a/matchms/filtering/add_precursor_mz.py
+++ b/matchms/filtering/add_precursor_mz.py
@@ -1,19 +1,9 @@
-from typing import Iterable
-from typing import TypeVar
+from matchms.utils import get_first_common_element
 from ..typing import SpectrumType
 
 
-_T = TypeVar('_T')
 _accepted_keys = ["precursor_mz", "precursormz", "precursor_mass"]
 _accepted_types = (float, str, int)
-_convertible_types = (str, int)
-
-
-def get_first_common_element(first: Iterable[_T], second: Iterable[_T]) -> _T:
-    """ Get first common element from two lists.
-    Returns 'None' if there are no common elements.
-    """
-    return next((item for item in first if item in second), None)
 
 
 def add_precursor_mz(spectrum_in: SpectrumType) -> SpectrumType:
diff --git a/matchms/filtering/add_retention.py b/matchms/filtering/add_retention.py
new file mode 100644
index 00000000..f5b38c5e
--- /dev/null
+++ b/matchms/filtering/add_retention.py
@@ -0,0 +1,108 @@
+from typing import Any
+from typing import List
+from typing import Optional
+from matchms.utils import filter_none
+from matchms.utils import get_common_keys
+from ..typing import SpectrumType
+
+
+_retention_time_keys = ["retention_time", "retentiontime", "rt", "scan_start_time", "RT_Query"]
+_retention_index_keys = ["retention_index", "retentionindex", "ri"]
+
+
+def safe_store_value(spectrum: SpectrumType, value: Any, target_key: str) -> SpectrumType:
+    """Helper function to safely store a value in the target key without throwing an exception, but storing 'None' instead.
+
+    Args:
+        spectrum (SpectrumType): Spectrum to which to add 'value' in 'target_key'.
+        value (Any): Value to parse into 'target_key'.
+        target_key (str): Name of the key in which to store the value.
+
+    Returns:
+        SpectrumType: Spectrum with added key.
+    """
+    if value is not None:   # one of accepted keys is present
+        value = safe_convert_to_float(value)
+    spectrum.set(target_key, value)
+    return spectrum
+
+
+def safe_convert_to_float(value: Any) -> Optional[float]:
+    """Safely convert value to float. Return 'None' on failure.
+
+    Args:
+        value (Any): Object to convert to float.
+
+    Returns:
+        Optional[float]: Converted float value or 'None' if conversion is not possible.
+    """
+    if isinstance(value, list) and len(value) == 1:
+        value = value[0]
+    try:
+        value = float(value)
+        rt = value if value >= 0 else None  # discard negative RT values
+    except ValueError:
+        print(f"{value} can't be converted to float.")
+        rt = None
+    return rt
+
+
+def _add_retention(spectrum: SpectrumType, target_key: str, accepted_keys: List[str]) -> SpectrumType:
+    """Add value from one of accepted keys to target key.
+
+    Args:
+        spectrum (SpectrumType): Spectrum from which to read the values.
+        target_key (str): Key under which to store the value.
+        accepted_keys (List[str]): List of accepted keys from which a value will be read (in order).
+
+    Returns:
+        SpectrumType: Spectrum with value from first accepted key stored under target_key.
+    """
+    common_keys = get_common_keys(spectrum.metadata.keys(), accepted_keys)
+    values_for_keys = filter_none([spectrum.get(key) for key in common_keys])
+    values = list(map(safe_convert_to_float, values_for_keys))
+    value = next(filter_none(values), None)
+
+    spectrum = safe_store_value(spectrum, value, target_key)
+    return spectrum
+
+
+def add_retention_time(spectrum_in: SpectrumType) -> SpectrumType:
+    """Add retention time information to the 'retention_time' key as float.
+    Negative values and those not convertible to a float result in 'retention_time'
+    being 'None'.
+
+    Args:
+        spectrum_in (SpectrumType): Spectrum with retention time information.
+
+    Returns:
+        SpectrumType: Spectrum with harmonized retention time information.
+    """
+    if spectrum_in is None:
+        return None
+
+    spectrum = spectrum_in.clone()
+
+    target_key = "retention_time"
+    spectrum = _add_retention(spectrum, target_key, _retention_time_keys)
+    return spectrum
+
+
+def add_retention_index(spectrum_in: SpectrumType) -> SpectrumType:
+    """Add retention index into 'retention_index' key if present.
+
+
+    Args:
+        spectrum_in (SpectrumType): Spectrum with RI information.
+
+    Returns:
+        SpectrumType: Spectrum with RI info stored under 'retention_index'.
+    """
+    if spectrum_in is None:
+        return None
+
+    spectrum = spectrum_in.clone()
+
+    target_key = "retention_index"
+    spectrum = _add_retention(spectrum, target_key, _retention_index_keys)
+    return spectrum
diff --git a/matchms/utils.py b/matchms/utils.py
index c12897a5..6bc394c4 100644
--- a/matchms/utils.py
+++ b/matchms/utils.py
@@ -1,4 +1,6 @@
 import re
+from typing import Iterable
+from typing import List
 from typing import Optional
 import numpy
 from .importing import load_adducts_dict
@@ -303,3 +305,35 @@ def clean_adduct(adduct: str) -> str:
 
     adduct_cleaned = adduct_core[:-len(adduct_charge)] + "]" + adduct_charge
     return adduct_conversion(adduct_cleaned)
+
+
+def get_first_common_element(first: Iterable[str], second: Iterable[str]) -> str:
+    """ Get first common element from two lists.
+    Returns 'None' if there are no common elements.
+    """
+    return next((item for item in first if item in second), None)
+
+
+def get_common_keys(first: List[str], second: List[str]) -> List[str]:
+    """Get common elements of two sets of strings in a case insensitive way.
+
+    Args:
+        first (List[str]): First list of strings.
+        second (List[str]): List of strings to search for matches.
+
+    Returns:
+        List[str]: List of common elements without regarding case of first list.
+    """
+    return [value for value in first if value in second or value.lower() in second]
+
+
+def filter_none(iterable: Iterable) -> Iterable:
+    """Filter iterable to remove 'None' elements.
+
+    Args:
+        iterable (Iterable): Iterable to filter.
+
+    Returns:
+        Iterable: Filtered iterable.
+    """
+    return filter(lambda x: x is not None, iterable)


Test patch:
diff --git a/tests/test_add_retention.py b/tests/test_add_retention.py
new file mode 100644
index 00000000..452f91f8
--- /dev/null
+++ b/tests/test_add_retention.py
@@ -0,0 +1,66 @@
+import numpy
+import pytest
+from matchms import Spectrum
+from matchms.filtering import add_retention_index
+from matchms.filtering import add_retention_time
+
+
+@pytest.mark.parametrize("metadata, expected", [
+    [{"retention_time": 100.0}, 100.0],
+    [{"retention_time": "NA"}, None],
+    [{"retention_time": "100.0"}, 100.0],
+    [{"retentiontime": 200}, 200.0],
+    [{"retentiontime": -1}, None],
+    [{"retentiontime": "-1"}, None],
+    [{"rt": 200}, 200.0],
+    [{"RT": 200}, 200.0],
+    [{"RT_Query": 200}, 200.0],
+    [{"nothing": "200"}, None],
+    [{'scan_start_time': 0.629566}, 0.629566],
+    [{'scan_start_time': [0.629566]}, 0.629566],
+    [{"rt": "None", "retentiontime": 12.17}, 12.17]
+])
+def test_add_retention_time(metadata, expected):
+    spectrum_in = Spectrum(mz=numpy.array(
+        [], "float"), intensities=numpy.array([], "float"), metadata=metadata)
+
+    spectrum = add_retention_time(spectrum_in)
+    actual = spectrum.get("retention_time")
+
+    if expected is None:
+        assert actual is None
+    else:
+        assert actual == expected and isinstance(actual, float)
+
+
+@pytest.mark.parametrize("metadata, expected", [
+    [{"retention_index": 100.0}, 100.0],
+    [{"retention_index": "NA"}, None],
+    [{"retention_index": "100.0"}, 100.0],
+    [{"retentionindex": 200}, 200.0],
+    [{"retentionindex": -1}, None],
+    [{"retentionindex": "-1"}, None],
+    [{"ri": 200}, 200.0],
+    [{"RI": 200}, 200.0],
+    [{"nothing": "200"}, None]
+])
+def test_add_retention_index(metadata, expected):
+    spectrum_in = Spectrum(mz=numpy.array(
+        [], "float"), intensities=numpy.array([], "float"), metadata=metadata)
+
+    spectrum = add_retention_index(spectrum_in)
+    actual = spectrum.get("retention_index")
+
+    if expected is None:
+        assert actual is None
+    else:
+        assert actual == expected and isinstance(actual, float)
+
+
+def test_empty_spectrum():
+    spectrum_in = None
+    spectrum = add_retention_time(spectrum_in)
+    assert spectrum is None, "Expected different handling of None spectrum."
+
+    spectrum = add_retention_index(spectrum_in)
+    assert spectrum is None, "Expected different handling of None spectrum."