BioSequences.jl icon indicating copy to clipboard operation
BioSequences.jl copied to clipboard

Implement broadcasting for BioSequences

Open TransGirlCodes opened this issue 2 years ago • 6 comments

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 avatar Jul 20 '21 12:07 TransGirlCodes

Codecov Report

Merging #171 (2865850) into v3 (a8e5b48) will decrease coverage by 0.18%. The diff coverage is 37.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##               v3     #171      +/-   ##
==========================================
- Coverage   82.16%   81.97%   -0.19%     
==========================================
  Files          31       32       +1     
  Lines        2159     2164       +5     
==========================================
  Hits         1774     1774              
- Misses        385      390       +5     
Flag Coverage Δ
unittests 81.97% <37.50%> (-0.19%) :arrow_down:

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

Impacted Files Coverage Δ
src/biosequence/seqbroadcast.jl 0.00% <0.00%> (ø)
src/longsequences/indexing.jl 93.54% <ø> (ø)
src/biosequence/biosequence.jl 82.05% <33.33%> (-9.38%) :arrow_down:
src/biosequence/predicates.jl 98.07% <100.00%> (ø)

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 a8e5b48...2865850. Read the comment docs.

codecov[bot] avatar Jul 20 '21 12:07 codecov[bot]

@jakobnissen Ok so, because broadcasting is a bit complex, I'm going to properly document every choice I make, and everything I've learned so far about the machinery here, as I make commits.

TransGirlCodes avatar Jul 20 '21 12:07 TransGirlCodes

Looks good! I'll follow from the sidelines :)

jakobnissen avatar Jul 20 '21 13:07 jakobnissen

If you get to understand how broadcasting works, can you perhaps check this method ambiguity out?

Base.Broadcast.BroadcastStyle(s1::BioSequences.PFMBroadcastStyle, s2::Base.Broadcast.BroadcastStyle) in BioSequences at /home/jakob/code/BioSequences.jl/src/search/pwm.jl:76

vs

Base.Broadcast.BroadcastStyle(::S, ::Base.Broadcast.Unknown) where S<:Base.Broadcast.BroadcastStyle in Base.Broadcast at broadcast.jl:133

Is this a runtime error waiting to happen or is it OK?

jakobnissen avatar Jul 24 '21 20:07 jakobnissen

I've cleared this from the v3.0 milestone, we're officially gonna push this back to a version 3.X.

TransGirlCodes avatar Oct 12 '21 14:10 TransGirlCodes

Can you rebase this on master? (no rush though)

jakobnissen avatar Jan 06 '22 09:01 jakobnissen