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

UpdateVcfSequenceDictionary cannot write index #863

Closed
nh13 opened this issue Jul 10, 2017 · 11 comments · Fixed by #867
Closed

UpdateVcfSequenceDictionary cannot write index #863

nh13 opened this issue Jul 10, 2017 · 11 comments · Fixed by #867
Assignees
Labels

Comments

@nh13
Copy link
Collaborator

nh13 commented Jul 10, 2017

@ronlevine looks like you introduced a bug, can you fix? Try writing it to a file with CREATE_INDEX=true. Since you wrapped it with an output stream :/. I am curious as to why the change was made at all, since shouldn't VariantContextWriterBuilder.determineOutputTypeFromFile handle this? What was the original problem?

https://github.com/broadinstitute/picard/blob/master/src/main/java/picard/vcf/UpdateVcfSequenceDictionary.java#L92-L99

@nh13
Copy link
Collaborator Author

nh13 commented Jul 10, 2017

@ronlevine even better, if the output file is output.vcf.gz, it doesn't write a gzipped file, but rather a text file named output.vcf.gz.

@nh13
Copy link
Collaborator Author

nh13 commented Jul 10, 2017

#741

@nh13
Copy link
Collaborator Author

nh13 commented Jul 10, 2017

@ronlevine I have added a potential fix into htsjdk so that setOutputFile should work: samtools/htsjdk#933

@ronlevine
Copy link
Contributor

ronlevine commented Jul 11, 2017

Reverted the non-test changes from #741 and fixed/expanded tests on https://github.com/broadinstitute/picard/compare/rhl_uvsd_fix_stdout. samtools/htsjdk#933 should fix the stdout test failure.

@nh13
Copy link
Collaborator Author

nh13 commented Jul 11, 2017

I checked out both your htsjdk and picard branches and got the following when testing it:

To get help, see http://broadinstitute.github.io/picard/index.html#GettingHelp
Exception in thread "main" java.lang.NullPointerException
	at htsjdk.variant.variantcontext.writer.VCFWriter.writeAndResetBuffer(VCFWriter.java:125)
	at htsjdk.variant.variantcontext.writer.VCFWriter.writeHeader(VCFWriter.java:136)
	at picard.vcf.UpdateVcfSequenceDictionary.doWork(UpdateVcfSequenceDictionary.java:94)
	at picard.cmdline.CommandLineProgram.instanceMain(CommandLineProgram.java:205)
	at picard.cmdline.PicardCommandLine.instanceMain(PicardCommandLine.java:94)
	at picard.cmdline.PicardCommandLine.main(PicardCommandLine.java:104)

@ronlevine
Copy link
Contributor

ronlevine commented Jul 11, 2017

The Picard branch should get that NPE since the htsjdk fix from nh_vcf_to_stdout is not yet in. Update nh_vcf_to_stdout and try again, it should work fine. nh_vcf_to_stdout runs without error locally for me as well as on travis, the automated testing framework.

@nh13
Copy link
Collaborator Author

nh13 commented Jul 11, 2017

Still getting the NPE. I checked out the branch in htsjdk, ran ./gradlew install printVersion, then ran $ ./gradlew shadowJar -Dhtsjdk.version=2.10.1-17-g269f0c1-SNAPSHOT in Picard. Can you send me an executable jar for me to verify?

@ronlevine
Copy link
Contributor

I am getting the same error. VCFWriter is trying to write a buffer to a null outputStream.This needs to be set. I added your original change back into htsjdk and am still getting the same error. The htsjdk snapshot is now 2.10.1-18-gc24bd07-SNAPSHOT.

@ronlevine
Copy link
Contributor

ronlevine commented Jul 11, 2017

I made a change in VariantContextWriter.build() so the VCF_STREAM case does not use a null stream:

final OutputStream writerStream = outStream != null ? outStream : outStreamFromFile;

The following command successfully wrote to stdout without error:
java -jar ./build/libs/picard.jar UpdateVcfSequenceDictionary INPUT=testdata/picard/vcf/vcfFormatTest.vcf OUTPUT=/dev/stdout SD=testdata/picard/vcf/dummy.reference.dict. I need to fix the Picard test for stdout because the output is not being captured to the file used for diffing.

@nh13
Copy link
Collaborator Author

nh13 commented Oct 16, 2017

@ronlevine can you take a look at this?

@yfarjoun
Copy link
Contributor

@cmnbroad feel like taking this one too?

@cmnbroad cmnbroad self-assigned this Oct 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants