-
Notifications
You must be signed in to change notification settings - Fork 77
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: upgrade sig overlap
and sig subtract
to load more than JSON signatures
#3153
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## latest #3153 +/- ##
==========================================
+ Coverage 86.64% 86.65% +0.01%
==========================================
Files 136 136
Lines 15807 15821 +14
Branches 2713 2713
==========================================
+ Hits 13696 13710 +14
Misses 1801 1801
Partials 310 310
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
for more information, see https://pre-commit.ci
sig overlap
and sig subtract
sig overlap
and sig subtract
Ready for review @sourmash-bio/devs |
sig overlap
and sig subtract
sig overlap
and sig subtract
to load more than JSON signatures
@ccbaumler @AnneliektH would either of you be able to look at this and (potentially) approve it? lmk if you don't have review privileges. |
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.
lgtm!
Note: PR into #3153 Tackles some signature loading and saving cleanup throughout the codebase. Most changes are in the tests, and this is a significant cleanup of the test code! Fixes #1062. --- Goals: * deprecate external use of `sourmash.signature` load/save functions, because they are JSON-specific and inflexible. * simplify and standardize signature load/save function usage during tests; * get rid of deprecation messages during tests; In brief, * rename `sourmash.signature.load_signatures` to `load_signatures_from_json`; * rename `sourmash.signature.load_one_signature` to `load_one_signature_from_json`; * rename `sourmash.signature.save_signatures` to `save_signatures_to_json`; * deprecate `sourmash.save_signatures` and `sourmash.load_one_signature` for 5.0 (joining `load_signatures`, which was already deprecated); * reduce/eliminate deprecations by transitioning internal test code to use these three functions directly from `sourmash.signature` instead of from the top-level sourmash import. * **bonus**: eliminate zipfile UserWarning around overwriting files, which causes lots of warnings when running tests. --- Done: - [x] in sourmash.signature submodule, rename `load_signatures` to `load_signatures_from_json`, `load_one_signature` to `load_one_signatures_from_json`, and `save_signatures` to `save_signatures_to_json`; make tests pass. - [x] deprecate `sourmash.load_one_signature` and `sourmash.save_signatures`. - [x] catch zipfile UserWarning for duplicate filenames in ZipStorage.save TODO: - [x] transition internal sourmash code+tests away from deprecated functions - [ ] create issue around changing API documentation prior to 5.0; --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Fix
sig overlap
andsig subtract
to take more than just JSON signatures.Also, adds a function
sourmash_args.load_one_signature
that I think should (eventually) replace the now-deprecatedsourmash.signature.load_one_signature
. This will be the topic of a new PR - for now, I think it's a nice quick fix!Fixes #3136
Related issues:
load_one_signature
useload_file_as_signatures
API #1062 - will do another PR to close this issueload_*
sig/db functions insourmash_args.py
#1877TODO: