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

Cannot open SAM file with bam::Reader #251

Closed
holtgrewe opened this issue Aug 31, 2020 · 3 comments · Fixed by #253
Closed

Cannot open SAM file with bam::Reader #251

holtgrewe opened this issue Aug 31, 2020 · 3 comments · Fixed by #253

Comments

@holtgrewe
Copy link
Contributor

It forces the file format to be BAM or CRAM.

@vsoch
Copy link
Contributor

vsoch commented Sep 3, 2020

heyo! I'm relatively new to rust and I'm hoping this is something I could try helping with. I've been able to clone recursively and run tests (all pass) and then I added a basic test for this case (is this what you are reporting to fail?)

    #[test]
    fn test_read_against_sam() {
        let mut bam = bam::Reader::from_path("./test/bam2sam_out.sam").unwrap();
        for read in bam.records() {
            let read = read.unwrap();
        }
    }

and then it fails.

    Finished test [unoptimized + debuginfo] target(s) in 0.10s
     Running target/debug/deps/rust_htslib-d59cbe136cb7cea4

running 1 test
test bam::ext::tests::test_read_against_sam ... FAILED

failures:

---- bam::ext::tests::test_read_against_sam stdout ----
thread 'bam::ext::tests::test_read_against_sam' panicked at 'called `Result::unwrap()` on an `Err` value: Open { target: "./test/bam2sam_out.sam" }', src/bam/ext.rs:9126:23
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Could you (or maybe @brainstorm can help) walk through the steps we should take to fix this? Just be safe to assume I know nothing :)

@holtgrewe
Copy link
Contributor Author

Hi, I think the point to start with would be here.

https://github.com/rust-bio/rust-htslib/blob/master/src/bam/mod.rs#L831

@brainstorm
Copy link
Member

Hey @vsoch!

I would start by getting your VSCode up and running with rust-analyzer, since it makes navigating and interacting with Rust projects a bit easier (IMHO), this pack of extensions is quite handy:

https://marketplace.visualstudio.com/items?itemName=jrobsonchase.rust-extension-pack

You were totally in the right path (thanks Manuel!) except the bam:: prefix for the Reader:

diff --git a/src/bam/mod.rs b/src/bam/mod.rs
index 1c7feef..76c622c 100644
--- a/src/bam/mod.rs
+++ b/src/bam/mod.rs
@@ -776,7 +776,8 @@ fn hts_open(path: &[u8], mode: &[u8]) -> Result<*mut htslib::htsFile> {
             unsafe {
                 // Comparison against 'htsFormatCategory_sequence_data' doesn't handle text files correctly
                 // hence the explicit checks against all supported exact formats
-                if (*ret).format.format != htslib::htsExactFormat_bam
+                if (*ret).format.format != htslib::htsExactFormat_sam
+                    && (*ret).format.format != htslib::htsExactFormat_bam
                     && (*ret).format.format != htslib::htsExactFormat_cram
                 {
                     return Err(Error::Open {
@@ -1103,6 +1104,14 @@ CCCCCCCCCCCCCCCCCCC"[..],
         assert_eq!(header_text, true_header);
     }

+    #[test]
+    fn test_read_against_sam() {
+        let mut bam = Reader::from_path("./test/bam2sam_out.sam").unwrap();
+        for read in bam.records() {
+            let _read = read.unwrap();
+        }
+    }
+
     fn _test_read_indexed_common(mut bam: IndexedReader) {
         let (names, flags, seqs, quals, cigars) = gold();
         let sq_1 = b"CHROMOSOME_I";

So please, ship it in the shape of a PR and welcome to the Bio rustaceans shore ;)

vsoch added a commit that referenced this issue Sep 7, 2020
Signed-off-by: vsoch <vsochat@stanford.edu>
vsoch added a commit that referenced this issue Sep 7, 2020
Signed-off-by: vsoch <vsochat@stanford.edu>
Rust-Htslib (doc|hac)kathon 2020 automation moved this from To do to Done Sep 7, 2020
brainstorm pushed a commit that referenced this issue Sep 7, 2020
Signed-off-by: vsoch <vsochat@stanford.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants