-
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
gCNV vcf cleanup #6352
gCNV vcf cleanup #6352
Conversation
Only called alleles as alts; QS to QUAL
80208b7
to
7f97fb1
Compare
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 have a couple of suggestions but otherwise this is great.
@@ -372,7 +374,8 @@ public Object onTraversalSuccess() { | |||
|
|||
private void generateIntervalsVCFFileFromAllShards() { | |||
logger.info("Generating intervals VCF file..."); | |||
final VariantContextWriter intervalsVCFWriter = createVCFWriter(outputIntervalsVCFFile); | |||
final VariantContextWriter intervalsVCFWriter = GATKVariantContextUtils.createVCFWriter( |
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.
Would it be better to use GATKTool
's createVCFWriter()
?
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.
The GATKTool method won't accept a sequence dictionary. It relies on the engine discovering a sequence dictionary from one of the inputs, which doesn't work so well here because the inputs are specified as directories.
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.
Isn’t the dictionary hooked up to createVCFWriter now via getBestAvailableSequenceDictionary?
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.
Ah yes, this is a holdover from before I implemented that suggestion Louis had. Thanks for the extra set of eyes.
@@ -412,7 +415,8 @@ private void generateSegmentsVCFFileFromAllShards() { | |||
|
|||
/* write variants */ | |||
logger.info(String.format("Writing segments VCF file to %s...", outputSegmentsVCFFile.getAbsolutePath())); | |||
final VariantContextWriter segmentsVCFWriter = createVCFWriter(outputSegmentsVCFFile); | |||
final VariantContextWriter segmentsVCFWriter = GATKVariantContextUtils.createVCFWriter( |
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.
Same here?
@@ -128,7 +128,7 @@ VariantContext composeVariantContext(final IntegerCopyNumberSegment segment) { | |||
int copyNumberCall = segment.getCallIntegerCopyNumberState().getCopyNumber(); | |||
|
|||
final VariantContextBuilder variantContextBuilder = new VariantContextBuilder(); | |||
variantContextBuilder.alleles(ALL_ALLELES); | |||
//variantContextBuilder.alleles(ALL_ALLELES); |
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.
Can delete
@@ -155,8 +155,14 @@ VariantContext composeVariantContext(final IntegerCopyNumberSegment segment) { | |||
genotypeBuilder.attribute(QSE, FastMath.round(segment.getQualityEnd())); | |||
final Genotype genotype = genotypeBuilder.make(); | |||
|
|||
final List<Allele> vcAlleles = new ArrayList<>(Collections.singletonList(REF_ALLELE)); |
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.
Could write this as final List<Allele> vcAlleles = Lists.newArrayList(REF_ALLELE);
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.
Lists
is in which package? avro?
Only list called ALTs
Set QUAL to QS (quality of some event), which we've found to be the best for filtering
Resolves most of the rest of #6167