use new load_file_as_signatures function more broadly
#1059
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1059 +/- ##
==========================================
+ Coverage 83.51% 92.51% +8.99%
==========================================
Files 99 74 -25
Lines 8885 5662 -3223
==========================================
- Hits 7420 5238 -2182
+ Misses 1465 424 -1041
Continue to review full report at Codecov.
|
ctb
changed the title
load_file_as_signatures function more broadly
|
ok, this looks ready for a full review. ...any takers? it's not really THAT many changes, apart from the new tests 😁 |
|
Let's try... |
sourmash/sourmash_args.py
Outdated
| sig_md5 = sig.md5sum() | ||
| if sig_md5.startswith(select_md5.lower()): | ||
| sl = [sig] | ||
| break |
There was a problem hiding this comment.
for large collections (databases?), should we check if there is more than one sig starting with the same MD5?
There was a problem hiding this comment.
yes, thanks for catching this!
I'm not sure what to do about this... for really large collections it could be quite slow. But then again, for large collections it's not clear you want to use this! So, yeah. I'll change the behavior and add a test.
There was a problem hiding this comment.
I'm not sure what to do about this... for really large collections it could be quite slow. But then again, for large collections it's not clear you want to use this! So, yeah. I'll change the behavior and add a test.
Yeah... at this point there is no access to the databases anymore, where this info can be checked while loading or it's less expensive (at least for SBTs? I think it is for LCA too).
#1044 added a new function
load_file_as_signaturesthat permittedsig splitto load signatures from databases as well as signature files. This PR deploys that function a bit more widely, and adds selector functionality.This PR permits many (most?) CLI functions to load signatures from databases without any special sauce.
So, for example
sourmash sig describecan now be run on SBTs and LCAs. It also addresses many generic loading issues, including stdin loading (#1049), loading from directories via subdir traversal, and the ability to provide lists of signature files in a text file as queries or database index builds.Examples
This will update
old.sigand add the newly computed signature(s) frominput.fa--This will concatenate all signatures under
tests/test-data--This will list all signatures in that database --
This will use the signature with that md5sum in that database as a query against all the signatures under that directory --
Testing thoughts
While the actual changes are relatively minor (no significant changes to current tests!) the implications are pretty big and so I'm planning on adding quite a few tests. We're also hitting on some untested behavior and/or grey areas in current behavior, so I'll add some lockdown tests to do my best to make that our current behavior for 3.x isn't too changed.
Affected issues
Fixes #1050 - load_signature now fails properly on non-existent file
Fixes #1048 - traverse-directory behavior now on by default in signature submodule commands
Fixes #1049 - stdin now accepted for signature input
Fixes #672 - compare now runs on SBTs/LCAs as well
Fixes #594 - sourmash sig extract now works on databases
Fixes #662 - added
--from-fileand--query-from-filein appropriate placesCloses #1039 - can convert b/t databases now, on command line
Fixes #978 - new function
load_file_as_indexnow loads databases as Index subclasses.Detailed list of new functionality
General functionality
Almost all commands that take a list of signatures can now load signatures from LCA and SBT databases. Commands that need a single query signature also support loading from databases, as well as signature files, and can usually use an
--md5selector to pick out the right signature.Expansion of support for reading signatures from stdin.
Specific additions to CLI
--from-filetosourmash compareCLI--md5selector tosourmash gatherandsourmash searchto pick a query signature.--from-filetosourmash index--query-from-filetosourmash lca classifyandsourmash lca summarize--from-filetosourmash lca index--query-from-filetosourmash multigather--md5selector tosourmash sig exportto pick a single signature--from-filetosourmash compareSpecific additions to API
load_file_as_indexis new top-level API for loadingIndexobjects from files.load_file_as_signaturesis new top-level API for loading of collections of signatures, including from SBT dbs, LCA dbs, and directories.Bug fixes
sourmash sig renamenow correctly fails on files that it could not open (+ tested)signature.load_signaturesbehavior wheredo_raiseis now respected (+ added test)TODO:
sourmash sig splitandsourmash sig cat#1048, do we need to provide an explicit --traverse-directory flag? maybe just traverse by default?sig renamethat did NOT useload_signatures(..., do_raise=True)load_one_signatureuseload_file_as_signaturesAPI #1062sourmash_args.load_query_signatureload_query_signatureload_dbs_and_sigsload_databasefunction to replaceload_sbtetc. ref provide a unified loading API for databases #978--from-fileto compare?sourmash_args.load_file_as_signatureserroretc calls insignature.load_signaturesTesting TODO
--traverse-directoryand--forcebehavior--md5behavior with query--md5behavior with queryindex--from-file; make issue re argparse modification stuff for 4.0 (adjust sourmash index argparse to accomodate --from-file #1066)lca classify--query-from-file, with and without other--query; traverse testslca summarize--query-from-file, with and without other--query; traverse testslca index --from-file; make issue re argparse modification for 4.0; traverse testsmultigather--query-from-filecomparenew behavior; traverse stuff, toomake testDid it pass the tests?make coverageIs the new code covered?without a major version increment. Changing file formats also requires a
major version number increment.
changes were made?