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

Add FASTAWriter and TwoBitWriter. #79

Merged
merged 1 commit into from
May 30, 2017
Merged

Conversation

alumi
Copy link
Member

@alumi alumi commented May 30, 2017

Summary

This PR adds (naive implementation of) writers for FASTA and 2bit format.

Changes

  • Add protocol ISequenceWriter to cljam.io.
  • Add cljam.fasta.writer.FASTAWriter and cljam.twobit.writer.TwoBitWriter.
  • Moved readers in cljam.twobit to cljam.twobit.reader.
  • Add function read-all-sequences to ISequenceReader.
  • Fix docstring of ISequenceReader.
  • Add tests.

Affected components

  • cljam.fasta
  • cljam.twobit

Notes

Performance is not yet optimized.

@codecov
Copy link

codecov bot commented May 30, 2017

Codecov Report

Merging #79 into master will increase coverage by 0.34%.
The diff coverage is 85.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #79      +/-   ##
==========================================
+ Coverage   82.07%   82.42%   +0.34%     
==========================================
  Files          59       62       +3     
  Lines        3922     4113     +191     
  Branches      423      439      +16     
==========================================
+ Hits         3219     3390     +171     
- Misses        280      284       +4     
- Partials      423      439      +16
Impacted Files Coverage Δ
src/cljam/fasta.clj 100% <100%> (ø) ⬆️
src/cljam/io.clj 100% <100%> (ø) ⬆️
src/cljam/fasta/core.clj 100% <100%> (ø) ⬆️
src/cljam/core.clj 97.29% <100%> (+0.32%) ⬆️
src/cljam/twobit.clj 76.92% <70%> (-3.95%) ⬇️
src/cljam/twobit/reader.clj 80% <80%> (ø)
src/cljam/twobit/writer.clj 87.96% <87.96%> (ø)
src/cljam/fasta/writer.clj 93.87% <93.87%> (ø)
... and 2 more

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 7da4841...809a57e. Read the comment docs.

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.

Thanks. Reflection warning is found. Please fix it and other trivial points.

@@ -0,0 +1,162 @@
(ns cljam.twobit.writer
(:require [clojure.java.io :as cio]
[clojure.string :as cstr]
Copy link
Member

Choose a reason for hiding this comment

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

cstr is not used in this ns.

([rdr]
(read-all-sequences rdr {}))
([rdr option]
(for [{:keys [name offset]} (.file-index rdr)]
Copy link
Member

Choose a reason for hiding this comment

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

Reflection warning is found.

[clojure.java.io :as cio]
[cljam.io :as io]
[cljam.lsb :as lsb]
[cljam.util :as util])
Copy link
Member

Choose a reason for hiding this comment

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

cstr and util are not used in this ns.

@alumi alumi force-pushed the feature/sequence-writers branch from b2a5cae to 809a57e Compare May 30, 2017 07:40
@alumi
Copy link
Member Author

alumi commented May 30, 2017

Thank you for your review!
I am sorry for my carelessness. I've fixed ns and installed linter...

@totakke totakke merged commit d5cf8f4 into master May 30, 2017
@totakke totakke deleted the feature/sequence-writers branch May 30, 2017 08:35
@totakke
Copy link
Member

totakke commented May 30, 2017

Good, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants