Skip to content

Conversation

@poshul
Copy link

@poshul poshul commented Dec 13, 2024

OpenMS' Percolator adapter saves the search_engine as Percolator directly. As a result it's not possible to load OpenMS generated pepxmls that have Percolator postprocessing into Skyline. This patch fixes that.

Copy link
Member

@chambm chambm left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. It needs a test added though. Easiest way would be taking one of the existing Percolated pepXMLs and changing the search_engine (and removing post-processor parameter if OpenMS doesn't set that). Better would be getting a real OpenMS pepXML but cutting it down to be <1MB. Is it clear enough how to add a test when looking at the tests/Jamfile.jam? (when you run the test the first time it will create a reference/xxx.observed file; just change that .check after reviewing it to be sure it's correct and then the test should pass).

(analysisType_ == MSGF_ANALYSIS && score_name == "qvalue") ||
(analysisType_ == CRUX_ANALYSIS && score_name == "percolator_qvalue")) {
(analysisType_ == CRUX_ANALYSIS && score_name == "percolator_qvalue")
(analysisType_ == PERCOLATOR_ANALYSIS)) {
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a missing || operator? How did this work?

Copy link
Author

Choose a reason for hiding this comment

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

And this is why I shouldnt push changes on a friday afternoon. Fixed.

@poshul
Copy link
Author

poshul commented Dec 16, 2024

Thanks for the PR. It needs a test added though. Easiest way would be taking one of the existing Percolated pepXMLs and changing the search_engine (and removing post-processor parameter if OpenMS doesn't set that). Better would be getting a real OpenMS pepXML but cutting it down to be <1MB. Is it clear enough how to add a test when looking at the tests/Jamfile.jam? (when you run the test the first time it will create a reference/xxx.observed file; just change that .check after reviewing it to be sure it's correct and then the test should pass).

I've been working on the test for this, a meaningful search with decoys, in a small mzml file, that produces better results than "nothing found" is proving to be a pain. I'm not going to have a chance to circle back to this before the holidays, so I'm putting a note in my calendar to come back in early Jan.

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.

2 participants