Repository: matchms/matchms
Pull request #701 (2024-07-15 15:45:51)

Increased Test Coverage


Patch:
diff --git a/matchms/exporting/metadata_export.py b/matchms/exporting/metadata_export.py
index dfbcfb42..bbae4be8 100644
--- a/matchms/exporting/metadata_export.py
+++ b/matchms/exporting/metadata_export.py
@@ -1,11 +1,15 @@
 import csv
 import json
+import logging
 from typing import Any, Dict, List, Optional, Set, Tuple
 import numpy as np
 from ..Spectrum import Spectrum
 from ..utils import filter_empty_spectra, rename_deprecated_params
 
 
+logger = logging.getLogger("matchms")
+
+
 def _get_metadata_dict(spectrum: Spectrum, include_fields: Optional[List[str]] = None) -> Dict[str, Any]:
     """Extract keys from spectrum metadata. Will silently continue if a key is not found.
 
@@ -19,7 +23,7 @@ def _get_metadata_dict(spectrum: Spectrum, include_fields: Optional[List[str]] =
     if include_fields is None or include_fields[0] == "all":
         return spectrum.metadata
     if not isinstance(include_fields, list):
-        print("'Include_fields' must be 'all' or list of keys.")
+        logger.warning("'Include_fields' must be 'all' or list of keys.")
         return None
 
     return {key: spectrum.metadata[key] for key in spectrum.metadata.keys()
@@ -61,8 +65,8 @@ def export_metadata_as_csv(spectra: List[Spectrum], filename: str,
         Expected input is a list of  :py:class:`~matchms.Spectrum.Spectrum` objects.
     filename:
         Provide filename to save metadata of spectrum(s) as csv file.
-    identifier:
-        Identifier used for naming each spectrum in the output file.
+    include_fields:
+        Columns to include.
     """
     spectra = filter_empty_spectra(spectra)
     metadata, columns = get_metadata_as_array(spectra)


Test patch:
diff --git a/tests/exporting/test_exporting_metadata.py b/tests/exporting/test_exporting_metadata.py
index 563fb616..00dc3f25 100644
--- a/tests/exporting/test_exporting_metadata.py
+++ b/tests/exporting/test_exporting_metadata.py
@@ -63,7 +63,7 @@ def test_export_metadata_as_json(tmp_path, spectra):
 
 
 @pytest.mark.parametrize("file_name", ["metadata.csv", "metadata.json"])
-def test_export_metadata_none_spectra(tmp_path, spectra, file_name):
+def test_export_metadata_none_spectra(tmp_path, spectra, file_name, caplog):
     outpath = os.path.join(tmp_path, file_name)
 
     spectra.append(None)
@@ -72,3 +72,10 @@ def test_export_metadata_none_spectra(tmp_path, spectra, file_name):
     with open(outpath, "r", encoding="utf-8") as f:
         actual = json.load(f)
         assert len(actual) == (len(spectra) - 1)
+
+    # Test invalid field export config
+    export_metadata_as_json(spectra, outpath, "invalid")
+    assert "'Include_fields' must be 'all' or list of keys." in caplog.text
+    with open(outpath, "r", encoding="utf-8") as f:
+        actual = json.load(f)
+        assert len(actual) == 0
diff --git a/tests/exporting/test_save_as_msp.py b/tests/exporting/test_save_as_msp.py
index c22d54cf..5e871bcb 100644
--- a/tests/exporting/test_save_as_msp.py
+++ b/tests/exporting/test_save_as_msp.py
@@ -1,3 +1,4 @@
+import logging
 import os
 import tempfile
 from typing import List
@@ -26,6 +27,7 @@ def data(request):
     spectra = load_test_spectra_file(request.param)
     return spectra
 
+
 def load_test_spectra_file(test_filename):
     module_root = os.path.join(os.path.dirname(__file__), "..")
     spectra_file = os.path.join(module_root, "testdata", test_filename)
@@ -56,8 +58,7 @@ def save_and_reload_spectra(filename, spectra: List[Spectrum], write_peak_commen
     reloaded_spectra = list(load_from_msp(filename))
     return reloaded_spectra
 
-
-def test_wrong_filename_exception():
+def test_wrong_filename_exception(caplog):
     """ Test for exception being thrown if output file doesn't end with .msp. """
     with tempfile.TemporaryDirectory() as temp_dir:
         filename = os.path.join(temp_dir, "test.mzml")
@@ -68,6 +69,14 @@ def test_wrong_filename_exception():
         message = exception.value.args[0]
         assert message == "File extension '.mzml' not allowed."
 
+        # Test warning log for filename except extensions not allowed
+        filename = os.path.join(temp_dir, "test.txt")
+
+        with caplog.at_level(logging.WARNING):
+            save_as_msp(None, filename)
+
+        assert "Spectrum(s) will be stored as msp file with extension .txt" in caplog.text
+
 
 # Using tmp_path fixture from pytest: https://docs.pytest.org/en/stable/tmpdir.html#the-tmp-path-fixture
 def test_file_exists_single_spectrum(spectrum, filename):
diff --git a/tests/exporting/test_save_spectra.py b/tests/exporting/test_save_spectra.py
index 4dba9bd2..c416db05 100644
--- a/tests/exporting/test_save_spectra.py
+++ b/tests/exporting/test_save_spectra.py
@@ -1,7 +1,9 @@
+import logging
 import os
+import re
 import tempfile
 import pytest
-from matchms.exporting import save_spectra
+from matchms.exporting.save_spectra import save_as_pickled_file, save_spectra
 from matchms.importing import load_from_mgf, load_spectra
 
 
@@ -17,7 +19,7 @@ def load_test_spectra_file():
                           "spectra.mgf",
                           "spectra.json",
                           "spectra.pickle"])
-def test_spectra(file_name):
+def test_spectra(file_name, caplog):
     """ Utility function to save spectra to msp and load them again.
 
     Params:
@@ -42,6 +44,46 @@ def test_spectra(file_name):
         reloaded_spectrum.set("num_peaks", None)
         assert spectrum == reloaded_spectrum
 
+    # Test file exists error
+    with tempfile.TemporaryDirectory() as temp_dir:
+        filename = os.path.join(temp_dir, file_name)
+
+        with open(filename, "w", encoding="utf-8") as file:
+            file.write("content")
+        assert os.path.exists(filename)
+
+        with pytest.raises(FileExistsError, match=re.escape(f"The specified file: {filename} already exists.")):
+            save_spectra(spectrum_list, filename)
+
+    # Test append to different filetype not supported error
+    ftype = os.path.splitext(file_name)[1].lower()[1:]
+    if ftype in ["json", "pickle"]:
+        with tempfile.TemporaryDirectory() as temp_dir:
+            filename = os.path.join(temp_dir, file_name)
+
+            with pytest.raises(ValueError, match=re.escape(f"{ftype} isn't supported for when `append` is True")):
+                save_spectra(spectra=spectrum_list, file=filename, append=True)
+
+    # Test logger warning when using pickle
+    if ftype == "pickle":
+        with caplog.at_level(logging.ERROR):
+            with tempfile.TemporaryDirectory() as temp_dir:
+                filename = os.path.join(temp_dir, file_name)
+                save_spectra(spectra=spectrum_list, file=filename, export_style="invalid")
+
+        assert "The only available export style for pickle is 'matchms', your export style invalid" in caplog.text
+
+
+def test_spectra_invalid_ext():
+    spectrum_list = load_test_spectra_file()
+
+    # Test invalid file ext
+    with tempfile.TemporaryDirectory() as temp_dir:
+        filename = os.path.join(temp_dir, "invalid.txt")
+
+        with pytest.raises(TypeError, match=re.escape(f"File extension of file: {filename} is not recognized")):
+            save_spectra(spectrum_list, filename)
+
 
 @pytest.mark.parametrize("file_name", ["spectra.pickle"])
 def test_save_as_pickled_file_none_spectra(file_name):
@@ -56,10 +98,26 @@ def test_save_as_pickled_file_none_spectra(file_name):
     reloaded_spectra: Spectra loaded from saved msp file.
     """
     spectrum_list = load_test_spectra_file()
-    spectrum_list.append(None)
 
+    # Test file exists error
     with tempfile.TemporaryDirectory() as temp_dir:
         filename = os.path.join(temp_dir, file_name)
-        save_spectra(spectrum_list, filename)
-        reloaded_spectra = list(load_spectra(filename))
-    assert len(reloaded_spectra) == (len(spectrum_list) - 1)
+
+        with open(filename, "w", encoding="utf-8") as file:
+            file.write("content")
+        assert os.path.exists(filename)
+
+        with pytest.raises(FileExistsError, match=re.escape(f"The file '{filename}' already exists.")):
+            save_as_pickled_file(spectrum_list, filename)
+
+    with tempfile.TemporaryDirectory() as temp_dir:
+        filename = os.path.join(temp_dir, file_name)
+
+        # Test no spectra list
+        with pytest.raises(TypeError, match="Expected list of spectra"):
+            save_as_pickled_file("invalid", filename)
+
+        # Test empty spectra list
+        spectrum_list = [None]
+        with pytest.raises(TypeError, match="Expected list of spectra"):
+            save_as_pickled_file(spectrum_list, filename)
diff --git a/tests/filtering/test_spectrum_processor.py b/tests/filtering/test_spectrum_processor.py
index 01ab5e8a..0ddffcf9 100644
--- a/tests/filtering/test_spectrum_processor.py
+++ b/tests/filtering/test_spectrum_processor.py
@@ -1,10 +1,14 @@
 import os
+import re
 import numpy as np
+import pandas as pd
 import pytest
 from matchms import SpectrumProcessor
 from matchms import filtering as msfilters
 from matchms.filtering.default_pipelines import BASIC_FILTERS
-from matchms.filtering.SpectrumProcessor import ProcessingReport
+from matchms.filtering.SpectrumProcessor import (ProcessingReport,
+                                                 create_partial_function,
+                                                 objects_differ)
 from matchms.importing.load_spectra import load_spectra
 from ..builder_Spectrum import SpectrumBuilder
 
@@ -70,6 +74,10 @@ def test_incomplete_parameters():
         processor = SpectrumProcessor(filters=())
         processor.parse_and_add_filter("require_correct_ionmode")
 
+    with pytest.raises(ValueError):
+        processor = SpectrumProcessor(filters=())
+        processor.parse_and_add_filter(("add_parent_mass", {'estimate_from_adduct': False}, "some_incorrect_param"))
+
 
 def test_string_output():
     processing = SpectrumProcessor(filters=["make_charge_int",
@@ -117,12 +125,22 @@ def test_filter_spectra_report(spectra):
     assert actual_masses == expected_masses
     assert report.counter_number_processed == 3
     assert report.counter_changed_metadata == {'make_charge_int': 2, 'interpret_pepmass': 3, 'derive_ionmode': 3}
-    report_df = report.to_dataframe()
-    assert np.all(report_df.loc[["require_minimum_number_of_peaks", "interpret_pepmass",
-                                 "correct_charge"]].values == np.array(
-        [[1, 0, 0],
-         [0, 3, 0],
-         [0, 0, 0]]))
+    expected_output = np.array([[1, 0, 0], [0, 3, 0], [0, 0, 0]])
+
+    if pd.__version__ >= '2.2.0':
+        # Test without pandas silent downcasting
+        report_df = report.to_dataframe()
+        assert np.all(report_df.loc[["require_minimum_number_of_peaks", "interpret_pepmass",
+                                     "correct_charge"]].values == expected_output)
+
+        # Test with pandas silent downcasting
+        pd.set_option("future.no_silent_downcasting", False)
+        report_df = report.to_dataframe()
+        assert np.all(report_df.loc[["require_minimum_number_of_peaks", "interpret_pepmass",
+                                     "correct_charge"]].values == expected_output)
+    else:
+        with pytest.raises(pd.errors.OptionError):
+            pd.get_option("future.no_silent_downcasting")
 
 
 def test_processing_report_class(spectra):
@@ -135,6 +153,9 @@ def test_processing_report_class(spectra):
     assert not processing_report.counter_removed_spectra
     assert processing_report.counter_changed_metadata == {"test_filter": 3}
 
+    expected_repr_parts = r"Report\(\d, *defaultdict\(<class 'int'>, {}\), *{}, *{'test_filter': 3}, *{}\)"
+    assert re.search(expected_repr_parts, repr(processing_report)) is not None
+
 
 def test_adding_custom_filter(spectra):
     def nonsense_inchikey(s):
@@ -368,3 +389,49 @@ def test_save_partial_spectra_spectrum_processor(ftype, should_work, spectra, tm
             assert spectrum.get("precursor_mz") is not None
     else:
         assert not os.path.exists(filename)
+
+
+@pytest.mark.parametrize("first, second, expected", [
+    (1, 1, False),
+    (1, 2, True),
+    ("a", "a", False),
+    ("a", "b", True),
+    ([1, 2, 3], [1, 2, 3], False),
+    ([1, 2, 3], [3, 2, 1], True),
+    (np.array([1, 2, 3]), np.array([1, 2, 3]), False),
+    (np.array([1, 2, 3]), np.array([3, 2, 1]), True),
+    (np.array([1, 2, 3]), np.array([1, 2, 3, 4]), True),
+    (np.array([1, 2, 3]), [1, 2, 3], False),
+    ([1, 2, 3], np.array([1, 2, 3]), False),
+    (np.array([1, 2, 3]), 1, True),
+    (np.array([1, 2, 3]), "a", True),
+    ("a", np.array([1, 2, 3]), True),
+    (1, np.array([1, 2, 3]), True),
+])
+def test_objects_differ(first, second, expected):
+    assert objects_differ(first, second) == expected
+
+
+@pytest.mark.parametrize(
+    "filter_params, expected_result, expected_exception",
+    [
+        ({"a": 2}, 5, None),
+        (None, 5, None),
+        ("invalid_param", None, ValueError),
+    ]
+)
+def test_create_partial_filter(filter_params, expected_result, expected_exception):
+    def sample_filter(a, b):
+        return a + b
+
+    if expected_exception:
+        with pytest.raises(expected_exception, match="Expected a dictionary for filter_args got"):
+            create_partial_function(sample_filter, filter_params)
+    else:
+        partial_func = create_partial_function(sample_filter, filter_params)
+        if filter_params:
+            assert partial_func(b=3) == expected_result
+        else:
+            assert partial_func(a=2, b=3) == expected_result
+
+        assert partial_func.__name__ == "sample_filter"