Skip to content

Fix error when calling multiple Spectra objects in addFragments#20

Merged
lgatto merged 60 commits intorformassspectrometry:mainfrom
guideflandre:main
Apr 8, 2025
Merged

Fix error when calling multiple Spectra objects in addFragments#20
lgatto merged 60 commits intorformassspectrometry:mainfrom
guideflandre:main

Conversation

@guideflandre
Copy link
Contributor

@guideflandre guideflandre commented Jan 22, 2025

This PR is linked to #18 and the discussed topics in this Spectra issue.

The motivation for this PR:

  1. addFragments used to throw an error when multiple spectra where called
  2. The new variable_modifications parameter necessitated a new addFragments function to distinguish annotations

The solution:
addFragments now returns a list of named elements (peptide sequence that include modifications) containing a character() vector with the annotations. Each element of the list has an attribute linking the annotations to the spectrum it belonged to.
For instance:

Expand example
> addFragments(sc[1:2], variable_modifications = c(I = 25.68))
$`DLEAHI[25.68]DSANK`
 [1] NA    NA    NA    NA    "y1_" NA    NA    NA    NA    NA    "y1"  NA    NA    NA    NA    NA   
[17] NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA   
[33] "b2"  NA    NA    NA    "y2*" "y2"  NA    NA    NA    NA    NA    NA    NA    NA    NA    NA   
[49] NA    NA    NA    "b3"  NA    NA    NA    NA    NA    NA    "y4"  NA    NA    NA    NA    NA   
[65] NA    NA    "y5"  NA    "b5"  NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA   
[81] NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA   
attr(,"spectrumNumber")
[1] 1

$DLEAHIDSANK
 [1] NA    NA    NA    NA    "y1_" NA    NA    NA    NA    NA    "y1"  NA    NA    NA    NA    NA   
[17] NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA   
[33] "b2"  NA    NA    NA    "y2*" "y2"  NA    NA    NA    NA    NA    NA    NA    NA    NA    NA   
[49] NA    NA    NA    "b3"  NA    NA    NA    NA    NA    NA    "y4"  NA    NA    NA    NA    NA   
[65] NA    NA    "y5"  NA    "b5"  NA    "y6"  NA    NA    "b6"  NA    NA    NA    "y7"  NA    "b7" 
[81] "y8"  NA    "b8_" "b8"  NA    NA    NA    "b9_" "y9"  NA    NA    NA   
attr(,"spectrumNumber")
[1] 1

$`TI[25.68]SETI[25.68]ER`
 [1] NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA  
[20] NA   "y1" NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA  
[39] NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA  
[58] NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA  
[77] NA   NA   NA   NA   NA   NA   NA   NA  
attr(,"spectrumNumber")
[1] 2

$`TI[25.68]SETIER`
 [1] NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA   
[17] NA    NA    NA    NA    "y1"  NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA   
[33] NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA   
[49] NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    "y3"  NA    NA    NA    NA   
[65] NA    NA    NA    NA    NA    NA    NA    NA    "y4"  "y5"  NA    NA    "y6_" NA    NA    "y6" 
[81] NA    NA    NA    NA   
attr(,"spectrumNumber")
[1] 2

$`TISETI[25.68]ER`
 [1] NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA  
[20] NA   "y1" NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   "b2" NA  
[39] NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA  
[58] NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA   NA  
[77] NA   NA   NA   NA   NA   NA   NA   NA  
attr(,"spectrumNumber")
[1] 2

$TISETIER
 [1] NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA   
[17] NA    NA    NA    NA    "y1"  NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA   
[33] NA    NA    NA    NA    "b2"  NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA   
[49] NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    NA    "y3"  NA    NA    NA    NA   
[65] NA    NA    NA    NA    NA    NA    NA    NA    "y4"  "y5"  NA    NA    "y6_" NA    NA    "y6" 
[81] NA    NA    NA    NA   
attr(,"spectrumNumber")
[1] 2

This PR can only be accepted if the corresponding PR in Spectra is accepted !! Otherwise, plotSpectra won't work.

guideflandre and others added 30 commits December 19, 2024 16:33
feat: calculateFragments2 
Provides modifications to the generated theoretical fragments
feat: tests for calculateFragments2
Updates based on reviewed PR
Updates according to reviewed PR
Co-authored-by: Sebastian Gibb <mail@sebastiangibb.de>
Corrected .cumsumFragmentMasses
Change modified peptide layout into AGC[57.02]AK instead of AG[C]AK to specify the modified mass
Merge branch 'main' of https://github.com/guideflandre/PSMatch

# Conflicts:
#	R/fragments-calculate2.R
@lgatto lgatto self-requested a review January 30, 2025 10:14
Copy link
Member

@sgibb sgibb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO the return value of addFragments is not correct or would be surprising in some cases.
BTW: maybe we should use this change to rename the addFragments function to labelFragments? or fragmentLabels or something else? addFragments suggests that some fragments where added to the Spectra object. Instead this function returns fragment labels.

@guideflandre
Copy link
Contributor Author

guideflandre commented Jan 31, 2025

BTW: maybe we should use this change to rename the addFragments function to labelFragments? or fragmentLabels or something else? addFragments suggests that some fragments where added to the Spectra object. Instead this function returns fragment labels.

Yes, this makes sense. Maybe add a warning that says addFragments is deprecated and labelFragments should be called instead ?

@lgatto
Copy link
Member

lgatto commented Feb 1, 2025

I am also in favour of adding a new function, labelFragments(), and deprecate addFragments() and make it call the new function:

addFragment <- function(x, tolerance = 0, ppm = 20, ...) {
    .Deprecated("labelFragments")
    labelFragments(x, tolevance, ppm, ...)
}

labelFragments <- function(x, tolerance = 0, ppm = 20, ...) {
    ## new code comes here
}

@guideflandre
Copy link
Contributor Author

I am also in favour of adding a new function, labelFragments(), and deprecate addFragments() and make it call the new function:

I added this as well. addFragments is now deprecated and labelFragments is called instead.

@lgatto
Copy link
Member

lgatto commented Apr 8, 2025

I have checked the package the reverse deps, and all is fine.

@lgatto lgatto merged commit 9d216a8 into rformassspectrometry:main Apr 8, 2025
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants