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

[MRG] add -A/--abundance-from to sig subtract & add sig inflate #1889

Merged
merged 13 commits into from
Mar 19, 2022

Conversation

ctb
Copy link
Contributor

@ctb ctb commented Mar 18, 2022

This PR adds -A/--abundance-from to the sourmash sig subtract command, per #1888.

It also:

  • implements implicit intersection in MinHash.inflate(...) in the situation where self is not a subset of from_mh
  • adds tests for this feature of MinHash.inflate(...)
  • cleans up the abundance borrowing code in sourmash sig intersect -A to use MinHash.inflate(...)
  • adds sourmash sig inflate

Fixes #1888
Fixes #1706

TODO:

@codecov
Copy link

codecov bot commented Mar 18, 2022

Codecov Report

Merging #1889 (606362e) into latest (dd5c013) will increase coverage by 8.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           latest    #1889      +/-   ##
==========================================
+ Coverage   82.64%   90.65%   +8.01%     
==========================================
  Files         121       92      -29     
  Lines       13159     8998    -4161     
  Branches     1774     1780       +6     
==========================================
- Hits        10875     8157    -2718     
+ Misses       2018      577    -1441     
+ Partials      266      264       -2     
Flag Coverage Δ
python 90.65% <100.00%> (+0.10%) ⬆️
rust ?

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

Impacted Files Coverage Δ
src/sourmash/cli/sig/__init__.py 100.00% <100.00%> (ø)
src/sourmash/cli/sig/inflate.py 100.00% <100.00%> (ø)
src/sourmash/cli/sig/subtract.py 100.00% <100.00%> (ø)
src/sourmash/minhash.py 92.56% <100.00%> (ø)
src/sourmash/sig/__main__.py 92.95% <100.00%> (+1.11%) ⬆️
src/core/src/ffi/signature.rs
src/core/src/errors.rs
src/core/tests/minhash.rs
src/core/src/ffi/index/revindex.rs
src/core/src/lib.rs
... and 27 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 dd5c013...606362e. Read the comment docs.

@ctb ctb changed the title [MRG] add -A/--abundance-from to sourmash sig subtract [WIP] add -A/--abundance-from to sig subtract & add sig inflate` Mar 18, 2022
@ctb
Copy link
Contributor Author

ctb commented Mar 18, 2022

@taylorreiter thoughts and suggestions welcome! I think it's mostly done, will put it up for review if when I look at it with fresh eyes it seems good (and all the code is covered).

@taylorreiter
Copy link
Contributor

ooooh sig inflate is really cool. The documentation is nice a clear too! I don't really have any suggestions, I think the clear documentation is the biggest key, as with that I think I can imagine different ways to get what I want using combinations of the sourmash sig commands. This is awesome!

@ctb ctb changed the title [WIP] add -A/--abundance-from to sig subtract & add sig inflate` [MRG] add -A/--abundance-from to sig subtract & add sig inflate` Mar 18, 2022
@ctb ctb changed the title [MRG] add -A/--abundance-from to sig subtract & add sig inflate` [MRG] add -A/--abundance-from to sig subtract & add sig inflate Mar 18, 2022
@ctb
Copy link
Contributor Author

ctb commented Mar 18, 2022

ok, this is now ready for review and merge @sourmash-bio/devs! No particular hurry.

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