-
Notifications
You must be signed in to change notification settings - Fork 7
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
Merge code for Kmer type from BioSequences to this package #8
Conversation
Author: BenJWard <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just given it a quick skim (too much code to deeply review) - it all looks great. I'm going to give it a test drive now :)
#@inline encoded_data_type(::Type{BigMer{A,K}}) where {A,K} = UInt128 | ||
#@inline encoded_data_type(x::AbstractMer) = encoded_data_type(typeof(x)) | ||
#@inline encoded_data(x::AbstractMer) = reinterpret(encoded_data_type(typeof(x)), x) | ||
#@inline ksize(::Type{T}) where {A,K,T<:AbstractMer{A,K}} = K | ||
#@inline Base.unsigned(x::AbstractMer) = encoded_data(x) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some commented out sections here, and below, and also a bunch of whitespace above this.
I should get moving on that BioSequences update. It seems like you lean on the interface for this package! |
I'd prefer if BioSequences hit v3 before I properly massage this to work with the new interface yeah. Then once this is a decent version based on v3, I can work on kmer-counting, spectras, and the genome assembly stuff again. |
I'm going to merge this just to get the code as it is onto master. Then we'll do a few PRs to massage the thing and get the docs and test coverage done for release. |
No description provided.