bittremieux / spectrum_utils

Python package for efficient mass spectrometry data processing and visualization
https://spectrum-utils.readthedocs.io/
Apache License 2.0
130 stars 21 forks source link

[BUG] Setting twice static modifications doubles the offset #25

Closed jspaezp closed 2 years ago

jspaezp commented 2 years ago

Hello there!

I was using the package and noted an unexpected behavior, where setting the static modifications twice in the code (inadvertedly) made the mass offsets of an aminoacid to double instead of keeping the provided mass

Dummy example with plots: https://colab.research.google.com/drive/1x913OBS6ieRPLY3H91cT_wopxGvCKdQy?usp=sharing

"Error" source in the code https://github.com/bittremieux/spectrum_utils/blob/c45c30930de2326d5a39e453fe8e7fc58e388892/spectrum_utils/spectrum.py#L26-L40

Expected behavior:

Setting twice the same static modifications for an amino acid would keep the first one, instead of doubling the offset of the mass

Possible solutions:

  1. Throw a warning when setting multiple static mods on the same aminoacid
    • This would in practice keep same the behavior but let the user know that the offset is being doubled
  2. Dont change the mass if the same offset is being used twice on the same aa without resetting it first
  3. add an additional argument to static_modification to decide how to handle those cases, whether the mass diff should over-write the current one or append to it.

Potential code change:

def static_modification(amino_acid: str, mass_diff: float) -> None:
    """
    Globally modify the monoisotopic mass of an amino acid to set a static
    modification.
    Parameters
    ----------
    amino_acid : str
        The amino acid whose monoisotopic mass is modified.
    mass_diff : float
        The *mass difference* to be added to the amino acid's original
        monoisotopic mass.
    """
    global aa_mass
    ############## >>>> ADDITION <<<<<< ###############
    if _aa_mass[amino_acid] != aa_mass[amino_acid]:
        warnings.warn(f"Amino acid {amino_acid} already has a mass offset of {aa_mass[amino_acid] - _aa_mass[amino_acid]}, the provided mass of {mass_diff} will be added to that")
    ############## >>>>END OF ADDITION <<<<<< ###############

    aa_mass[amino_acid] += mass_diff

let me know what you think Kindest wishes Sebastian

bittremieux commented 2 years ago

Thanks for reporting the issue, that's indeed something I overlooked. I'll have it fixed in a new release soon.

bittremieux commented 2 years ago

Fixed in 84af89a.

Static modifications should no longer be set explicitly. Instead, the ProForma mechanism is to be used for specifying any type of modification.