-
Notifications
You must be signed in to change notification settings - Fork 596
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 Funcotator VCF output renderer to keep B37 contig names in the VCF output file #8539
Conversation
src/main/java/org/broadinstitute/hellbender/tools/funcotator/FuncotatorEngine.java
Show resolved
Hide resolved
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.
@jamesemery Looks reasonable, but I'm not sure we have adequate test coverage on the new code
src/main/java/org/broadinstitute/hellbender/tools/funcotator/Funcotator.java
Outdated
Show resolved
Hide resolved
src/main/java/org/broadinstitute/hellbender/tools/funcotator/FuncotatorEngine.java
Show resolved
Hide resolved
@droazen back to you |
@@ -924,7 +923,10 @@ protected void enqueueAndHandleVariant(final VariantContext variant, final Refer | |||
|
|||
final FuncotationMap funcotationMap = funcotatorEngine.createFuncotationMapForVariant(variant, referenceContext, featureContext); | |||
|
|||
// This is necessary because we want to revert the variant contig name change if it was applied in the FuncotatorEngine::getCorrectVariantContextForReference method before outputting the vcf. |
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.
This comment is still misleading -- can you make it clear that we don't always want to revert the variant contig name change, only in the very specific case where the input is b37 and the reference is hg19?
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.
👍 Looks good now, merge when tests pass @jamesemery
6ab2a84
to
78eddbc
Compare
… names on output for B37 aligned files (#8539)
Fixes a longstanding issue that resulted in mismatching sequence dictionary/vcf lines for B37 data provided to funcotator.