Repository: matchms/matchms

Issue:
`normalize_intensities` sets Spectrum to None
**Describe the bug**
*Not a bug, but unexpected behavior*
I noticed that `matchms:normalize_intensities` will set a `Spectrum` object with all peak intensities 0 to `None`. 

`WARNING:matchms:normalize_intensities:Spectrum with all peak intensities <= 0 was set to None.`

This is unexpected, since the `load_from_mgf` function will create a `Spectrum` object even if all peak intensities are 0. I would rather expect an error during `Spectrum` instantiation if such a condition is present. Having `normalize_intensities` set the `Spectrum` object to `None` is really disruptive, since it is a cleanup function of which I would not expect such an aggressive behavior. Usually, the `Spectrum` objects are in a list, and if there are suddenly `None` types included, this leads to `'NoneType' object has no attribute 'peaks'`.

**To Reproduce**
Steps to reproduce the behavior:
Assume a .mgf-file such as
```
BEGIN IONS
PEPMASS=1.0
COMPOUND_NAME=Fakeomycin
MSLEVEL=2
1234.12 0
1234.12 0
END IONS
```
```
spectra = list(
    matchms.importing.load_from_mgf(filename)
)
spectra = [matchms.filtering.normalize_intensities(i) for i in spectra]
print(spectra)
```
```
2025-03-07 19:47:59,207:WARNING:matchms:normalize_intensities:Spectrum with all peak intensities <= 0 was set to None.
[None]
```


**Expected behavior**
I would expect the Spectrum object initiation to fail during reading of the mgf file. It would be also okay to simply set the intensities to 0, or set Spectrum.peaks.intensities to an empty list.


**Desktop (please complete the following information):**
 - OS: Ubuntu 22.04
 - MatchMS version 0.28.2


**Suggestions**

I would propose the following changes:

- Include additional validation functions during `Spectrum` initiation (check if there are any ions, check if intensities != 0) and raise an error right away
- Change the behavior of `normalize_intensities()`

Let me know what you think. I have a bit of time coming up an can open a PR if desired.

Discussion:
@niekdejonge: Thanks @mmzdouc for the clear issue! Good point, that is indeed unexpected for such a filter.
@Knerten0815 is fixing it in #750 

The solution that is and easy to implement and prevents your pipeline from breaking, would be returning the Spectrum object, but without any peaks. Would this also be an intuitive way of handling this edge case to you? 

@niekdejonge: The reason for not just returning the peaks with intensity 0, is that this is not what we expect after normalization, so if we keep intensities of 0, it will result in breaking when calculating some scores. E.g. it will result in a division by 0 if we calculate the cosine score. @mmzdouc: @niekdejonge @Knerten0815 thanks for the quick response! 

This looks like a good fix allowing to do e.g. spectral library matching without running into an `AttributeError`.

I still think that implementing a validation function in the `__init__` of the `matchms.Spectrum` object would be the way to go to raise e.g. `RuntimeError` in case `Fragments.mz` or `Fragments.intensities` == `[]` or all values are `0` or negative. In my opinion, spectra without fragments or nonsensical values do not make sense to begin with, so catching them asap prevents downstream errors. In `fermo_core`, I address this with the following code, but perhaps this could be done in `matchms` right away?


```python
        self.stats.spectral_library = []
        for spectrum in mgf_gen:
            try:
                if len(spectrum.peaks.mz) == 0:
                    logger.warning(
                        f"SpecLibMgfParser: spectrum {spectrum.metadata.get('compound_name')} has no ions - SKIP"
                    )
                elif all(x <= 0 for x in spectrum.peaks.intensities):
                    logger.warning(
                        f"SpecLibMgfParser: all fragment ions of spectrum {spectrum.metadata.get('compound_name')} are <= 0 - SKIP"
                    )
                elif not spectrum.metadata.get("precursor_mz"):
                    logger.warning(
                        f"SpecLibMgfParser: spectrum {spectrum.metadata.get('compound_name')} has no pepmass/precursor m/z - SKIP"
                    )
                elif (
                    spectrum.metadata.get("precursor_mz") == 0.0
                    or spectrum.metadata.get("precursor_mz") == 1.0
                ):
                    logger.warning(
                        f"SpecLibMgfParser: pepmass/precursor m/z of spectrum {spectrum.metadata.get('compound_name')} are <= 1 - SKIP"
                    )
                else:
                    self.stats.spectral_library.append(
                        matchms.filtering.add_precursor_mz(
                            matchms.filtering.normalize_intensities(spectrum)
                        )
                    )
            except Exception as e:
                logger.warning(f"SpecLibMgfParser: {e}")
```@niekdejonge: Yes I agree that these type of spectra are not really a useful thing to store. However, we don't want to raise an error during loading, since this can be very frustrating when you load a few 100.000 spectra and it breaks halfway. So I would prefer a little bit less strict implementation. 

One thing that is easy to do now already in matchms is to run the filter require_minimum_number_of_peaks filter with n_required = 1.  So this will set every spectrum without peaks to None. If you use SpectrumProcessor or Pipeline, these None spectra are automatically removed and it logs how many were removed at each filtering step. 

Solution patch:
diff --git a/matchms/filtering/peak_processing/normalize_intensities.py b/matchms/filtering/peak_processing/normalize_intensities.py
index 1cf3a4d1..b80ac38e 100644
--- a/matchms/filtering/peak_processing/normalize_intensities.py
+++ b/matchms/filtering/peak_processing/normalize_intensities.py
@@ -21,8 +21,9 @@ def normalize_intensities(spectrum_in: SpectrumType) -> SpectrumType:
     max_intensity = np.max(spectrum.peaks.intensities)
 
     if max_intensity <= 0:
-        logger.warning("Spectrum with all peak intensities <= 0 was set to None.")
-        return None
+        logger.warning("Peaks of spectrum with all peak intensities <= 0 were deleted.")
+        spectrum.peaks = Fragments(mz=np.array([]), intensities=np.array([]))
+        return spectrum
 
     # Normalize peak intensities
     mz, intensities = spectrum.peaks.mz, spectrum.peaks.intensities


Test patch:
diff --git a/tests/filtering/test_normalize_intensities.py b/tests/filtering/test_normalize_intensities.py
index e5713d62..4b149525 100644
--- a/tests/filtering/test_normalize_intensities.py
+++ b/tests/filtering/test_normalize_intensities.py
@@ -52,8 +52,9 @@ def test_normalize_intensities_all_zeros(caplog):
 
     spectrum = normalize_intensities(spectrum_in)
 
-    assert spectrum is None, "Expected spectrum to be set to None."
-    msg = "Spectrum with all peak intensities <= 0 was set to None."
+    assert len(spectrum.peaks.intensities) == 0, "Expected no peak intensities."
+    assert len(spectrum.peaks.mz) == 0, "Expected no m/z values. "
+    msg = "Peaks of spectrum with all peak intensities <= 0 were deleted."
     assert msg in caplog.text, "Expected log message."