TransGirlCodes commented 2 years ago

Types of changes

This PR implements the following changes: (Please tick any or all of the following that are applicable)

[X] :sparkles: New feature (A non-breaking change which adds functionality).
[ ] :bug: Bug fix (A non-breaking change, which fixes an issue).
[ ] :boom: Breaking change (fix or feature that would cause existing functionality to change).

:ballot_box_with_check: Checklist

[ ] :art: The changes implemented is consistent with the julia style guide.
[ ] :blue_book: I have updated and added relevant docstrings, in a manner consistent with the documentation styleguide.
[ ] :blue_book: I have added or updated relevant user and developer manuals/documentation in docs/src/.
[ ] :ok: There are unit tests that cover the code changes I have made.
[ ] :ok: The unit tests cover my code changes AND they pass.
[ ] :pencil: I have added an entry to the [UNRELEASED] section of the manually curated CHANGELOG.md file for this repository.
[ ] :ok: All changes should be compatible with the latest stable version of Julia.
[ ] :thought_balloon: I have commented liberally for any complex pieces of internal code.

TransGirlCodes commented 2 years ago

@jakobnissen Would you like to have a play with this and check it is performant for your use case?

If it's good I'll include it in the docs and add tests.

codecov[bot] commented 2 years ago

Codecov Report

Merging #16 (dd48785) into master (f8ea398) will increase coverage by 0.34%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #16      +/-   ##
==========================================
+ Coverage   84.79%   85.14%   +0.34%     
==========================================
  Files          12       12              
  Lines         467      478      +11     
==========================================
+ Hits          396      407      +11     
  Misses         71       71

Flag	Coverage Δ
unittests	`85.14% <100.00%> (+0.34%)`	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/indexing.jl	`94.44% <100.00%> (+8.73%)`	:arrow_up:

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 f8ea398...dd48785. Read the comment docs.

jakobnissen commented 2 years ago

The generated code is beautiful - but the implementation seems wrong.

This doesn't seem to work for larger kmers:

julia> kmer = Kmer("TATAGCGCGGACATAGTCGTCGCTGCTTATATATATGCTGCTCTCGTCGTGTATAGAGAGAGATATCGGATCGATCGCTATGCGGATA")
DNA 88-mer:
TATAGCGCGGACATAGTCGTCGCTGCTTATATATATGCT…TGTATAGAGAGAGATATCGGATCGATCGCTATGCGGATA

julia> kmer[1:2]
ERROR: ArgumentError: itr does not contain enough elements (3 ≠ 2)
Stacktrace:
 [1] DNAKmer{2, 1}(itr::Tuple{UInt64, UInt64, UInt64})
   @ Kmers ~/.julia/packages/Kmers/7SNBQ/src/kmer.jl:139
 [2] getindex(seq::DNAKmer{88, 3}, i::UnitRange{Int64})
   @ Main ./REPL[18]:5
 [3] top-level scope
   @ REPL[40]:1

BioJulia / Kmers.jl

Kmer Slicing #16

Types of changes

:ballot_box_with_check: Checklist

Codecov Report