Improve performance for reading string from BCF.

[niyarin]

It took a long time to read ref data with a large BCF, so it needs to be improved.

[codecov]

Codecov Report

Merging #186 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #186      +/-   ##
==========================================
- Coverage   86.64%   86.59%   -0.05%     
==========================================
  Files          76       76              
  Lines        6011     6011              
  Branches      499      501       +2     
==========================================
- Hits         5208     5205       -3     
- Misses        304      305       +1     
- Partials      499      501       +2

Impacted Files	Coverage Δ
src/cljam/io/bcf/reader.clj	88.7% <100%> (-1.62%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ddfbec0...0e07efc. Read the comment docs.

[alumi]

Thank you for the improvement!
Would you add a test case that has string genotype fields with n-sample > 1 in cljam.io.bcf.reader-test?

e.g.

{:chr 0, :pos 1, :id "FOO;BAR", :ref "AAAC", :ref-length 4, :alt ["A"],
 :qual nil, :filter nil, :info [],
 :n-sample 2, :genotype [[1 [["FOOBAR"] ["BAZ"]]]
                         [2 [["FOO"] ["BAR" "BAZ"]]]
                         [3 [["FOO" "BAR" "BAZ"] [nil]]]]}

You can check the uncompressed binary representation by something like the following command.

echo -e '##fileformat=VCFv4.3\n##contig=<ID=1>\n##FORMAT=<ID=XX,Type=String,Number=.,Description="">\n##FORMAT=<ID=YY,Type=String,Number=.,Description="">\n##FORMAT=<ID=ZZ,Type=String,Number=.,Description="">\n#CHROM\tPOS\tID\tREF\tALT\tQUAL\tFILTER\tINFO\tFORMAT\tNORMAL\tTUMOR\n1\t1\tFOO;BAR\tAAAC\tA\t.\t.\t.\tXX:YY:ZZ\tFOOBAR:FOO:FOO,BAR,BAZ\tBAZ:BAR,BAZ' | bcftools view --no-version -Ou | hexdump -s 0x15b -C

[alumi]

Use case for comparing to constants, especially constant primitive values.
https://github.com/bbatsov/clojure-style-guide#case-vs-condcondp

[niyarin]

Thank you for pointing.
I fixed them.

[yito88]

LGTM 👍
Thank you for the change!

[alumi]

I'm sorry I didn't explain it enough.
I want tests in cljam.io.bcf.reader-test not to depend on I/O like other existing cases.
Would you remove two-samples.bcf and add hexdumped data to reader_test.clj?

echo -e '##fileformat=VCFv4.3\n##contig=<ID=1>\n##FORMAT=<ID=XX,Type=String,Number=.,Description="">\n##FORMAT=<ID=YY,Type=String,Number=.,Description="">\n##FORMAT=<ID=ZZ,Type=String,Number=.,Description="">\n#CHROM\tPOS\tID\tREF\tALT\tQUAL\tFILTER\tINFO\tFORMAT\tNORMAL\tTUMOR\n1\t1\tFOO;BAR\tAAAC\tA\t.\t.\t.\tXX:YY:ZZ\tFOOBAR:FOO:FOO,BAR,BAZ\tBAZ:BAR,BAZ' | bcftools view --no-version -Ou | hexdump -s 0x15b -e '/1 "0x%02x" " "' -v

diff --git test/cljam/io/bcf/reader_test.clj test/cljam/io/bcf/reader_test.clj
index 530c5db..5f9e0dc 100644
--- test/cljam/io/bcf/reader_test.clj
+++ test/cljam/io/bcf/reader_test.clj
@@ -50,6 +50,37 @@
      :qual 1.0, :filter [0], :info [[0 [1]] [10 [300]]],
      :n-sample 2, :genotype [[0 [[0] [1]]] [1 [[16] [32]]]]}
 
+    [0x28 0x00 0x00 0x00
+     0x3d 0x00 0x00 0x00
+     0x00 0x00 0x00 0x00
+     0x00 0x00 0x00 0x00
+     0x04 0x00 0x00 0x00
+     0x01 0x00 0x80 0x7f
+     0x00 0x00 0x02 0x00
+     0x02 0x00 0x00 0x03
+     0x77
+     0x46 0x4f 0x4f 0x3b 0x42 0x41 0x52
+     0x47 0x41 0x41 0x41 0x43
+     0x17 0x41
+     0x00
+     0x11 0x01
+     0x67
+     0x46 0x4f 0x4f 0x42 0x41 0x52
+     0x42 0x41 0x5a 0x00 0x00 0x00
+     0x11 0x02
+     0x87
+     0x46 0x4f 0x4f 0x00 0x00 0x00 0x00 0x00
+     0x42 0x41 0x52 0x2c 0x42 0x41 0x5a 0x00
+     0x11 0x03
+     0xc7
+     0x46 0x4f 0x4f 0x2c 0x42 0x41 0x52 0x2c 0x42 0x41 0x5a 0x00
+     0x2e 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00]
+    {:chr 0, :pos 1, :id "FOO;BAR", :ref "AAAC", :ref-length 4, :alt ["A"],
+     :qual nil, :filter nil, :info [],
+     :n-sample 2, :genotype [[1 [["FOOBAR"] ["BAZ"]]]
+                             [2 [["FOO"] ["BAR" "BAZ"]]]
+                             [3 [["FOO" "BAR" "BAZ"] [nil]]]]}
+
     [0x58 0x00 0x00 0x00
      0x00 0x00 0x00 0x00
      0x02 0x00 0x00 0x00

[niyarin]

Sorry,I misunderstood.
I fixed it.

[alumi]

Thanks for the update! 👍 Added some more trivial comments about the code style.

[alumi]

The local binding results is unnecessary now.

     (case type-id
       0 (repeat n-sample nil)
       7 (->> #(bytes->strs (lsb/read-bytes rdr total-len))
              (repeatedly n-sample)
              doall)
       (->> #(read-typed-atomic-value rdr type-id)
            (repeatedly (* n-sample total-len))
            (partition total-len)
            (map (fn [xs] (take-while #(not= % :eov) xs)))
            doall)))))

[niyarin]

Thank you for pointing.I fixed them.

[niyarin]

I fixed expression.

[alumi]

LGTM 👍 Thanks!!

[niyarin]

Thank you for reviewing!