8000 [Feature] id filter on consensusXML by jpfeuffer · Pull Request #4798 · OpenMS/OpenMS · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[Feature] id filter on consensusXML #4798

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

Merged
merged 2 commits into from
Jun 25, 2020

Conversation

jpfeuffer
Copy link
Contributor

No description provided.

Julianus Pfeuffer added 2 commits June 25, 2020 21:22
By extracting IDs, saving their featureID and mapping back. I did not check if EVERY filter criterion
makes sense for cXML but most of them (especially simple ones) should.
@timosachsenberg
Copy link
Contributor

nice!

@timosachsenberg timosachsenberg merged commit d7cc95f into OpenMS:develop Jun 25, 2020
Copy link
Contributor
@cbielow cbielow left a comment

Choose a reason for hiding this comment

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

sorry.
A bit late to the party, but could you have a look at again, please :)

for (auto&& p : f.getPeptideIdentifications())
{
p.setMetaValue("feature_id", id);
peptides.emplace_back(p);
Copy link
Contributor

Choose a reason for hiding this comment

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

Requires std::move (or std::forward if you stick to auto&&).
Also emplace_back suggests emplacing a new type, but the type stays the same so push_back is enough.

std::back_inserter(peptides));
unassigned.clear();

std::swap(proteins, cmap.getProteinIdentifications());
Copy link
Contributor

Choose a reason for hiding this comment

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

Move assignment is enough? No need to swap???

@@ -705,7 +738,31 @@ class TOPPIDFilter :
<< peptides.size() << " spectra identified with "
<< IDFilter::countHits(peptides) << " spectrum matches." << endl;

IdXMLFile().store(outputfile_name, proteins, peptides);
if (FileHandler::getTypeByFileName(inputfile_name) == FileTypes::IDXML)
Copy link
Contributor
@cbielow cbielow Jun 25, 2020

Choose a reason for hiding this comment

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

Why not getType ? Would also cover content if suffix is ambiguous.
But query once to avoid duplicate work.

{
UInt64 id = f.getUniqueId();
id_to_featureref[id] = &f;
for (auto&& p : f.getPeptideIdentifications())
Copy link
Contributor

Choose a reason for hiding this comment

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

I find this confusing, since there is no case where we use reference proxies in our code (that I'm aware of).
Use auto& p: ..?

}
else if (FileHandler::getTypeByFileName(inputfile_name) == FileTypes::CONSENSUSXML)
{
for (auto&& p : peptides)
Copy link
Contributor

Choose a reason for hiding this comment

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

same with auto&& and emplace here

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
0