Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small changes for FlashLFQ writer #131

Merged
merged 2 commits into from
Dec 4, 2024

Conversation

unholyparrot
Copy link
Contributor

I am using mokapot (v0.10.0) as an embedded rescoring engine within the ms2rescore package (https://github.com/compomics/ms2rescore, v3.0.3), which you may have heard of for rescoring results from sage (https://github.com/lazear/sage, v0.14.6).

I've tried doing label-free quantification with FlashLFQ GUI (v1.2.6), as you kindly provided the writer for its "Generic" format, and bumped into zero intensity for all quantified proteins. Checking the results.sage.ms2rescore.mokapot.flashlfq and *.raw files with FreeStyle, I found the unnecessary subdivision of the retention time for conversion to FlashLFQ compatible format, as the retention time is supposed to be minutes, but it's already in minutes.

To test the theory, I manually deleted the division in the source files of ms2rescore venv in lib/python3.10/site-packages/mokapot/writers/flashlfq.py at line 115 and resubmitted the job to ms2rescore and later to FlashLFQ.

As a result, all proteins are now appropriately quantified.

Please consider this PR if you find it useful.

Fixed retention time division by 60.
Time is required in minutes for FlashLFQ, it's already in minutues
@jspaezp
Copy link
Collaborator

jspaezp commented Dec 1, 2024

Humm I am not 100% sure if it is a valid assumption that the RT from sage will always be in seconds. Would you mind leaving the details on how the mzml was generated?

@unholyparrot
Copy link
Contributor Author

According to sage documentation, retention time is given in minutes.

mzml was generated with ThermoRawFileParser1.4.3 in command line mode with default parameters.

@jspaezp
Copy link
Collaborator

jspaezp commented Dec 2, 2024

Cool!

for future reference: https://github.com/lazear/sage/blob/73eaf49a4e53179b9bd5329bc1f085835f5f3987/crates/sage-cloudpath/src/mzml.rs#L250-L258

Sage does seem to check the unit at parse time of the mzml to make the units minutes. So I guess that if we specify in the mokapot docs that the rt column is meant to be in minutes, we could go ahead and do that. (not sure if other tools that write .pin files will default to minutes ...)

https://github.com/lazear/sage/blob/73eaf49a4e53179b9bd5329bc1f085835f5f3987/crates/sage-cloudpath/src/tdf.rs#L35 + MannLabs/timsrust#34 also makes sure that .d files are in seconds in sage.

@wfondrie do you recall why that /60 is there? is there any tool that writes the .pin in seconds (I think we should be able to remove it safely ...)?

@wfondrie wfondrie self-requested a review December 4, 2024 21:19
Copy link
Owner

@wfondrie wfondrie left a comment

Choose a reason for hiding this comment

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

I think this should be good. We're in the midst of some other large updates, so I'm going to go ahead and merge it.

@wfondrie wfondrie merged commit 0d1f437 into wfondrie:main Dec 4, 2024
1 of 10 checks passed
jspaezp pushed a commit to jspaezp/mokapot that referenced this pull request Dec 5, 2024
Fixed retention time division by 60.
Time is required in minutes for FlashLFQ, it's already in minutues

Co-authored-by: William Fondrie <fondriew@gmail.com>
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