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

order_and_orient: if some contigs align ambiguously, warn but still process the rest #760

Merged
merged 4 commits into from
Jan 23, 2018

Conversation

notestaff
Copy link
Contributor

No description provided.

@notestaff notestaff self-assigned this Jan 23, 2018
@dpark01
Copy link
Member

dpark01 commented Jan 23, 2018

I'll try to look at this, but with a few other things on the plate, it'd help to know if this PR was motivated by a bug that a user was running into or whether it was self-motivated cleanup.

@notestaff
Copy link
Contributor Author

I ran into this on one of kjsiddle's HIV samples when experimenting with SPAdes.

@dpark01
Copy link
Member

dpark01 commented Jan 23, 2018

Ok, and just to be clear, the previous behavior was to fail/error hard, and the new behavior is to drop such contigs (with a warning) and carry on?

@notestaff
Copy link
Contributor Author

Yes, i.e. to assemble rest of reference. On the example I saw, the hole left by the missing contig is then filled by gapfilling. Root cause, I think, was that SPAdes output a misassembled contig which glued together two non-adjacent pieces of reference, so the contig aligned to both; then it ends up with two alignments to reference. Could add a step to break up such misassemblies, as in SHIVER and QUAST.

@dpark01
Copy link
Member

dpark01 commented Jan 23, 2018

That sounds pretty good -- I think historically the AmbiguousAlignmentException was pretty rare, but it makes sense that it would pop up more with something like HIV, and that, in some of those scenarios, you'd have reason to suspect misassembly anyway.

Copy link
Member

@dpark01 dpark01 left a comment

Choose a reason for hiding this comment

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

This is a nice "surgical" PR.

@@ -15,6 +15,7 @@
import tempfile
import argparse
import itertools
import gzip
Copy link
Member

Choose a reason for hiding this comment

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

I think this import can be avoided... test_assembly.py already imports util.file which provides open_or_gzopen which is the more common pattern for opening gzip files.

@notestaff
Copy link
Contributor Author

this was actually LASV not HIV.

@notestaff notestaff merged commit 06b6cd6 into master Jan 23, 2018
@dpark01 dpark01 deleted the is-drop-ambig-aligns branch January 30, 2018 13:58
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