-
Notifications
You must be signed in to change notification settings - Fork 78
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] Accept file list in sourmash sig cat
#1236
Conversation
78e9975
to
2e80d7f
Compare
Codecov Report
@@ Coverage Diff @@
## latest #1236 +/- ##
==========================================
+ Coverage 83.29% 92.72% +9.43%
==========================================
Files 103 76 -27
Lines 9588 5977 -3611
==========================================
- Hits 7986 5542 -2444
+ Misses 1602 435 -1167
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
On Thu, Nov 05, 2020 at 01:25:52PM -0800, Luiz Irber wrote:
@luizirber commented on this pull request.
> @@ -402,6 +402,21 @@ def _load_database(filename, traverse_yield_all, *, cache_size=None):
except Exception as exc:
pass
+ if not loaded: # try load signatures from single file (list of signature paths)
+ try:
+ db = []
+ with open(filename, 'rt') as fp:
+ for line in fp:
+ line = line.strip()
+ if line:
+ sig = sourmash.load_signatures(line, quiet=True, do_raise=True)
I implemented it in `_load_database`, but the file list could potentially contains indices too (not only paths to signatures). Should this be a test case? (it wouldn't be too hard to change, I could change this line to use `load_file_as_signatures` instead...)
sure! I was thinking about doing that Back When but just got lazy :)
|
sourmash sig cat
sourmash sig cat
Ready for review and merge |
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.
Looks good. I think there are probably some edge cases that we will discover later ;)
* Allow loading DBs from a file list * add test and impl for passing DBs in the file list
Current behavior of
sourmash sig cat
doesn't support passing a file list as input. I would like to runsourmash sig cat filelist.txt
and be able to load signatures fromfilelist.txt
(one signature path per line).I think
load_file_as_signatures
(called here) is missing the call toload_file_list_of_signatures
?Checklist
make test
Did it pass the tests?make coverage
Is the new code covered?without a major version increment. Changing file formats also requires a
major version number increment.
changes were made?