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

For reduce worktime of lein cloverage #69

Merged
merged 5 commits into from Apr 28, 2017

Conversation

ayamada
Copy link
Contributor

@ayamada ayamada commented Apr 26, 2017

  • Reduce too many number of span in pileup test
  • Mark bamreader-medium-file test as deftest-slow (It may causes decreasing of coverage a bit)
  • Refine about-bam-indexer-small-file test

@codecov
Copy link

codecov bot commented Apr 26, 2017

Codecov Report

Merging #69 into master will decrease coverage by 1%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
- Coverage   82.62%   81.62%   -1.01%     
==========================================
  Files          55       55              
  Lines        3281     3281              
  Branches      361      366       +5     
==========================================
- Hits         2711     2678      -33     
- Misses        209      237      +28     
- Partials      361      366       +5
Impacted Files Coverage Δ
src/cljam/lsb.clj 79.31% <0%> (-10.35%) ⬇️
src/cljam/bam_index/chunk.clj 70% <0%> (-7.5%) ⬇️
src/cljam/bam/decoder.clj 81.11% <0%> (-4.9%) ⬇️
src/cljam/bam/writer.clj 73.13% <0%> (-3.74%) ⬇️
src/cljam/util/sam_util.clj 88.11% <0%> (-1.64%) ⬇️
src/cljam/util.clj 77.46% <0%> (-1.41%) ⬇️
src/cljam/bam/reader.clj 84.96% <0%> (-0.76%) ⬇️

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 8f62777...878f0fd. Read the comment docs.

{:chr "chr1" :start 23000000 :end 24000000 :depth :deep} 10010
{:chr "chr1" :start 23000000 :end 23500000 :depth :deep} 3806
{:chr "*"} 0)))))
Copy link
Member

Choose a reason for hiding this comment

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

These are tests for #21. Please confirm that numbers of spans (regions in BAM file) differ.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review.
I saw #21 and https://travis-ci.org/chrovis/cljam/jobs/193304007#L419 , I understood.
Restore original numbers of spans, and remove unrelated tests by d5d350f .

Copy link
Member

@alumi alumi Apr 28, 2017

Choose a reason for hiding this comment

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

How about this? I think it'll be faster!

        (are [?param ?counts] (= (count (io/read-alignments r ?param)) ?counts)
          {:chr "chr1" :start 23000000 :end 23001000 :depth :deep} 46 ;; 1 span
          {:chr "chr1" :start 24900000 :end 24902000 :depth :deep} 3  ;; 2 spans
          {:chr "chr1" :start 24000000 :end 24001000 :depth :deep} 6  ;; 3 spans
          {:chr "chr1" :start 23260000 :end 23268650 :depth :deep} 58 ;; 4 spans
          {:chr "chr1" :start 23430000 :end 23470000 :depth :deep} 55 ;; 5 spans
          {:chr "*"} 0) ;; 1 span

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I confirmed to test this parameters in old f14f070 , and got errors in 4 and 5 spans, but current HEAD don't get errors.
I applied it by 878f0fd .
Thank you!

…ts / Remove unneeded `:depth :shallow` `:depth :pointer` tests
Copy link
Member

@totakke totakke left a comment

Choose a reason for hiding this comment

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

Pretty good except a point of @alumi comments.

Copy link
Member

@alumi alumi left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! 👍

@alumi alumi merged commit f6ec43b into chrovis:master Apr 28, 2017
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.

None yet

3 participants