UpdateVcfSequenceDictionary should handle stdin and stdout gracefully #507
Comments
|
@yfarjoun Can you either give me picard write permission or assign me to this task? It would be much easier if I had write permission, I'd prefer to work in a branch rather than a fork. |
|
I'm pretty sure you have right access. |
|
@yfarjoun I do not have write access. I tried pushing my branch and got the following: I am not listed as a member: https://github.com/broadinstitute/picard/network/members |
|
@nh13 I can see a use case for stdout but is there one for stdin? |
|
Yes, streaming support would be desirable as I don't see any reason it would need to read the input twice. |
|
are you using the ssh link?
…On Fri, Jan 27, 2017 at 6:31 PM, Ron Levine ***@***.***> wrote:
@nh13 <https://github.com/nh13> I can see a use case for stdout but is
there one for stdin?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#507 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACnk0gRboiCp8TCayK2WlSAVIqwmHtoGks5rWn42gaJpZM4IFyOL>
.
|
|
I'm not sure what you are referring to by the "ssh link". I cloned picard then cd'ed to the picard directory I created a branch and could not push it to the origin repository. |
|
use the other link to clone: [email protected]:broadinstitute/picard.git
…On Fri, Jan 27, 2017 at 8:45 PM, Ron Levine ***@***.***> wrote:
I'm not sure what you are referring to by the "ssh link".
I cloned picard
https://github.com/broadinstitute/picard.git
MITAdmins-Air:picard ronlevine$ git remote -v
origin https://github.com/broadinstitute/picard.git (fetch)
origin https://github.com/broadinstitute/picard.git (push)
stable ***@***.***:broadinstitute/gsa-stable.git (fetch)
stable ***@***.***:broadinstitute/gsa-stable.git (push)
unstable ***@***.***:broadinstitute/gsa-unstable.git (fetch)
unstable ***@***.***:broadinstitute/gsa-unstable.git (push)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#507 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACnk0v93aBEMC6fxonoa00tl9hQsGjyRks5rWp2kgaJpZM4IFyOL>
.
|
|
Cloned |
|
try now...
…On Sat, Jan 28, 2017 at 9:31 AM, Ron Levine ***@***.***> wrote:
Cloned ***@***.***:broadinstitute/picard.git, created a branch, made
changes then tried to push them, but was denied
MITAdmins-Air:picard ronlevine$ git push -u origin rhl_uvsd_stdin_stdout_507
Enter passphrase for key '/Users/ronlevine/.ssh/id_rsa':
ERROR: Permission to broadinstitute/picard.git denied to ronlevine.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#507 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACnk0t61Hq4zEYhCkDQKgr78eH2LQ9pOks5rW1E9gaJpZM4IFyOL>
.
|
|
@nh13 Can you give me an explicit example of a command line using |
|
Should be simple to make one up using cat or the like. |
|
Testing streaming can be tricky when you aren't actually building a
command-line.
You could look at my recent test of MergeBamAlignment as an example of how
to stream into a test.
let me know if that doesn't help.
…On Sun, Jan 29, 2017 at 4:15 PM, Ron Levine ***@***.***> wrote:
@nh13 <https://github.com/nh13> Can you give me an explicit example of a
command line using stdin? I assume the following is the stout case:
java -jar ./build/libs/picard.jar UpdateVcfSequenceDictionary I=input.vcf
SD=sequencDictionay.vcf O=/dev/stdout
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#507 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACnk0knZXwlY0L6qahHNQcGZ8ktpI9epks5rXQF5gaJpZM4IFyOL>
.
|
|
@yfarjoun I grabbed the following lines of code and substituted
|
|
That probably means that it will not work in streaming..is the file being
opened again (for extracting the dictionary for example)?
That's the part that needs fixing.
…On Tue, Jan 31, 2017 at 11:53 AM, Ron Levine ***@***.***> wrote:
@yfarjoun <https://github.com/yfarjoun> I grabbed the following lines of
code and substituted InputVCFfIle.vcf
FileInputStream stream = new FileInputStream(InputVCFfIle.vcf);
final Field fdField = FileDescriptor.class.getDeclaredField("fd");
fdField.setAccessible(true);
final File inputStream = new File("/dev/fd/" + fdField.getInt(stream.getFD()));
VCFFileReader successfully reads the header during it's construction but
it's iterator is unable to read the rest of the file.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#507 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACnk0i-kxTNo0YgRmgLhcOErecYhlQuCks5rX2cjgaJpZM4IFyOL>
.
|
|
Exactly. HTSJDK's |
|
yeah...it might be harder to fix...there are several issues. fix what you
can, and leave the rest in a new issue, perhaps.
…On Tue, Jan 31, 2017 at 1:08 PM, Ron Levine ***@***.***> wrote:
Exactly. HTSJDK's TribbleIndexedFeatureReader.readHeader, which is used
by the VCFFileReader ctor closes it. For streaming, it can not be closed
then opened again. It's done that same way for the sequence dictionary.
And, there is a file type check for the sequence dictionary, which will
fail for this approach since the file name would be /dev/fd/*number*.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#507 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACnk0tiJeTosFl_ReP0Sp92EAtSJt0Slks5rX3i1gaJpZM4IFyOL>
.
|
|
That's the plan. Handling stdout is trivial. I think the stdin issues such as having the option to not close the stream after reading the header and determining the file type by examining the first line of the file instead of by file extension should be done in HTSJDK. |
|
I agree.
…On Tue, Jan 31, 2017 at 4:09 PM, Ron Levine ***@***.***> wrote:
That's the plan. Handling stdout is trivial. I think the stdout issues
such as having the option to not close the stream after reading the header
and determining the file type by examining the first line of the file
instead of by file extension should be done in HTSJDK.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#507 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACnk0lSTgqsUdl-T7N4Ly5ztpE5Evy08ks5rX6MFgaJpZM4IFyOL>
.
|
|
There is an existing HTSJDK issue, Add InputStream support to VCFFileReader. |
|
Listen, I may be off base here but can I ask why we don't have a separate centralizing I/O actor, like BWA does? Given the larger file sizes now, would this type of feature make sense? If not, why not? |
|
Does it handle input streams? It would have to decode that stream type on the fly. samtools/htsjdk#837 is handling the input stream. Can you point me to the BWA I/O actor? |
|
Someplace in https://github.com/lh3/bwa. Sorry I can't be more specific. |
|
Adding a comment that HTSJDK has tried to add this functionality (samtools/htsjdk#1245) but there are issues with the reader that still persist (#1776). The issue linked specifies |
Currently it will blow up trying to build the writer:
The text was updated successfully, but these errors were encountered: