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

re-enable reading bam files from pipes #1085

Merged
merged 2 commits into from
Feb 8, 2018
Merged

Conversation

lbergelson
Copy link
Member

fixing an issue that prevented reading a bam file from an unseekable file, ex: a unix pipe
fixes #1083 which was introduced by #1077

this only fixes the case where the bam is being opened as file, a similar issue exists for Paths, #1084
support for reading pipes as paths has never worked

Checklist

  • Code compiles correctly
  • New tests covering changes and new functionality
  • All tests passing
  • Extended the README / documentation, if necessary
  • Is not backward compatible (breaks binary or source compatibility)

@codecov-io
Copy link

codecov-io commented Feb 8, 2018

Codecov Report

Merging #1085 into master will increase coverage by 0.004%.
The diff coverage is 80%.

@@               Coverage Diff               @@
##              master     #1085       +/-   ##
===============================================
+ Coverage     66.179%   66.183%   +0.004%     
  Complexity      7635      7635               
===============================================
  Files            535       535               
  Lines          32394     32401        +7     
  Branches        5510      5512        +2     
===============================================
+ Hits           21438     21444        +6     
- Misses          8790      8792        +2     
+ Partials        2166      2165        -1
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/htsjdk/samtools/SamInputResource.java 67.407% <80%> (+0.22%) 15 <1> (ø) ⬇️
...rc/main/java/htsjdk/samtools/SamReaderFactory.java 64.356% <0%> (+0.495%) 7% <0%> (ø) ⬇️

fixing an issue that prevented reading a bam file from an unseekable file, ex: a unix pipe
fixes #1083 which was introduced by #1077

this only fixes the case where the bam is being opened as file, a similar issue exists for Paths, #1084
support for reading pipes as paths has never worked
@@ -335,7 +347,7 @@ public URL asUrl() {

@Override
public SeekableStream asUnbufferedSeekableStream() {
return lazySeekableStream.get();
return lazySeekableStream.get();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra whitespace

}

@Override
public InputStream asUnbufferedInputStream() {
return asUnbufferedSeekableStream();
final SeekableStream seekableStream = asUnbufferedSeekableStream();
if( seekableStream != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after if and not before seekableStream

return lazySeekableStream.get();
//if the file doesn't exist, the try to open the stream anyway because users might be expecting the exception
//if it not a regular file than we won't be able to seek on it, so return null
if(!fileResource.exists() || fileResource.isFile()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after if

Copy link
Contributor

@jacarey jacarey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a couple style nitpicks.. otherwise 👍

@lbergelson lbergelson merged commit 2b8919b into master Feb 8, 2018
@lbergelson lbergelson deleted the lb_fix_fifo_regression branch February 8, 2018 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

opening a SamReader on a named pipe is broken in 2.14.2
4 participants