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.

Sign up for GitHub

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

Jump to bottom

Add ms2rescore module, remove ms2pip and deeplc modules #293

Merged
merged 15 commits into from
Dec 18, 2023
Merged

Add ms2rescore module, remove ms2pip and deeplc modules #293

merged 15 commits into from
Dec 18, 2023

Conversation

jonasscheid
Copy link
Collaborator

@jonasscheid jonasscheid commented Nov 29, 2023 •
edited
Loading

Addressing #288

  • Adding MS2Rescore and deprecating MS2PIP and DeepLC modules, since they are incorporated in MS2Rescore
  • Adding a couple of TODOs for an upcoming refactoring
  • Removed test_full from gh actions since this should be only used on the release PR. In the next refactoring iteration, a smaller sized quantification test will be implemented to cover the quant subworkflow
  • Some correction on citations and further documentation

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/mhcquant branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 29, 2023 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit e9a7527

+| ✅ 158 tests passed       |+
!| ❗   5 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in WorkflowMhcquant.groovy: Optionally add in-text citation tools to this list.
  • system_exit - System.exit in WorkflowMain.groovy: System.exit(1) [line 51]
  • system_exit - System.exit in WorkflowMhcquant.groovy: System.exit(1) [line 18]
  • system_exit - System.exit in WorkflowMhcquant.groovy: System.exit(1) [line 45]
  • system_exit - System.exit in WorkflowMhcquant.groovy: System.exit(1) [line 51]

✅ Tests passed:

Run details

  • nf-core/tools version 2.10
  • Run at 2023-12-07 13:38:06

marissaDubbelaar
marissaDubbelaar approved these changes Nov 30, 2023
View reviewed changes
modules/local/openms_idfilter.nf Show resolved Hide resolved
modules/local/openms_idscoreswitcher.nf Show resolved Hide resolved
subworkflows/local/quant.nf Show resolved Hide resolved
@jonasscheid jonasscheid added enhancement New feature or request do-not-merge labels Dec 3, 2023
@jonasscheid
Copy link
Collaborator Author

There is an issue with adding extra features via psmfeatureextractor:
If ms2rescore removes peptides for some reason (e.g. ms2pip does not support selenocysteine (U)), then the features of ms2pip are not present in the peptideHit. Unfortunately, psmfeatureextractor removes features that are not present in ALL peptideHits, meaning all ms2pip features are not added as extra features.
We need to change psm_utils such that when updating peptideHits with rescoring features we make sure we remove every peptideHit that is not present in the psm_list of ms2rescore. Or we apply a filter within the ms2rescore_cli script

@jonasscheid jonasscheid self-assigned this Dec 3, 2023
marissaDubbelaar
marissaDubbelaar approved these changes Dec 6, 2023
View reviewed changes
Copy link
Collaborator

@marissaDubbelaar marissaDubbelaar left a comment

Choose a reason for hiding this comment

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

LGTM

@steffenlem steffenlem self-requested a review December 7, 2023 08:51
steffenlem
steffenlem requested changes Dec 7, 2023
View reviewed changes
Copy link
Contributor

@steffenlem steffenlem left a comment

Choose a reason for hiding this comment

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

As far as I can see, ms2rescore makes the RT prediction on the merged idXMLs of multiple runs. Is this intentional?

workflows/mhcquant.nf Show resolved Hide resolved
@steffenlem
Copy link
Contributor

My test run should be finished tomorrow

@jonasscheid jonasscheid merged commit d00d9b5 into nf-core:dev Dec 18, 2023
9 checks passed
@jonasscheid jonasscheid mentioned this pull request Dec 18, 2023
Closed
@jonasscheid jonasscheid mentioned this pull request May 30, 2024
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marissaDubbelaar marissaDubbelaar marissaDubbelaar approved these changes

@steffenlem steffenlem steffenlem approved these changes

Assignees

@jonasscheid jonasscheid

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jonasscheid @steffenlem @marissaDubbelaar