-
Notifications
You must be signed in to change notification settings - Fork 597
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
Fixed a bug where force calling alleles were lost upon trimming by placing allele injection after trimming #7679
Conversation
…acing allele injection after trimming
df52231
to
5a98a80
Compare
@ldgauthier The Travis failures were unrelated and went away after rebasing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix and great cleanup! I'd like you to add one comment for clarity, but otherwise LGTM!
@@ -599,6 +599,11 @@ public ActivityProfileState isActive( final AlignmentContext context, final Refe | |||
} | |||
|
|||
final SortedSet<VariantContext> allVariationEvents = untrimmedAssemblyResult.getVariationEvents(hcArgs.maxMnpDistance); | |||
for (final VariantContext given : givenAlleles) { | |||
if (allVariationEvents.stream().noneMatch(vc -> vc.getStart() == given.getStart() && vc.getAlternateAllele(0).basesMatch(given.getAlternateAllele(0)))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These will always be trimmed right? (And always biallelic?) Because they're each for a single haplotype? If you agree, then let's add a comment for posterity.
ReadThreadingAssembler.addAssembledVariantsToEventMapOutput(untrimmedAssemblyResult, assembledEventMapVariants, MTAC.maxMnpDistance, assembledEventMapVcfOutputWriter); | ||
|
||
final SortedSet<VariantContext> allVariationEvents = untrimmedAssemblyResult.getVariationEvents(MTAC.maxMnpDistance); | ||
for (final VariantContext given : givenAlleles) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love the duplication here, but there's not enough to merit refactoring.
* @param altAllele new allele, the bases of which replace those of refAllele | ||
* @param insertLocation location in the genome at which the new allele starts | ||
* | ||
* Example: suppose this haplotype starts at position 100 on its contig and has bases ACCGTTATATCG and we wish to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this -- very clear!
Closes #7672.
@ldgauthier This is the bug fix for Sarah Calvo. The allele was lost when trimming caused its haplotype to start with a deletion. The solution is to inject force-calling alleles after trimming. Are you able to review?