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] Refactor the database loading code in sourmash_args #1373

Merged
merged 6 commits into from
Mar 9, 2021

Conversation

ctb
Copy link
Contributor

@ctb ctb commented Mar 6, 2021

While spelunking in the codebase, I noticed that only the first element of the 3-tuple in the list returned by sourmash_args.load_dbs_and_sigs was being used by everything but SBTs. This cleans that up.

Note that this PR adds a .location attribute to SBTs to compensate for newly missing information. This is needed for one specific test, tests/test_sourmash.py::test_gather_csv. It seems harmless to me...

Checklist

  • Is it mergeable?
  • make test Did it pass the tests?
  • make coverage Is the new code covered?
  • Did it change the command-line interface? Only additions are allowed
    without a major version increment. Changing file formats also requires a
    major version number increment.
  • Was a spellchecker run on the source code and documentation after
    changes were made?

@codecov
Copy link

codecov bot commented Mar 6, 2021

Codecov Report

Merging #1373 (e7a13a3) into latest (5e66db9) will increase coverage by 5.29%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           latest    #1373      +/-   ##
==========================================
+ Coverage   88.84%   94.13%   +5.29%     
==========================================
  Files         123       96      -27     
  Lines       18264    14653    -3611     
  Branches     1409     1409              
==========================================
- Hits        16226    13794    -2432     
+ Misses       1800      621    -1179     
  Partials      238      238              
Flag Coverage Δ
python 94.13% <100.00%> (+<0.01%) ⬆️
rust ?

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

Impacted Files Coverage Δ
src/sourmash/sbt.py 84.30% <100.00%> (+0.06%) ⬆️
src/sourmash/search.py 91.22% <100.00%> (ø)
src/sourmash/sourmash_args.py 91.92% <100.00%> (ø)
src/core/tests/test.rs
src/core/src/from.rs
src/core/src/index/sbt/mhbt.rs
src/core/src/index/bigsi.rs
src/core/src/wasm.rs
src/core/tests/minhash.rs
src/core/src/errors.rs
... and 20 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 5e66db9...9a471c6. Read the comment docs.

ctb added a commit that referenced this pull request Mar 6, 2021
@ctb
Copy link
Contributor Author

ctb commented Mar 6, 2021

@luizirber - this PR touches on the SBT code. Ready for review!

src/sourmash/sbt.py Outdated Show resolved Hide resolved
src/sourmash/search.py Outdated Show resolved Hide resolved
ctb and others added 2 commits March 8, 2021 17:19
@ctb ctb merged commit b6c28d8 into latest Mar 9, 2021
@ctb ctb deleted the refactor/databases branch March 9, 2021 01:20
luizirber added a commit that referenced this pull request May 10, 2021
…etch` functionality. (#1370)

* more refactor - filename stuff

* add 'location' to SBT objects

* finish removing filename

* fix prefetch after merging in #1373

* implement a CounterGatherIndex

* remove sort

* update counter logic to remove proper intersection

* make 'find' a generator

* remove comment

* begin refactoring 'categorize'

* have the 'find' function for SBTs return signatures

* fix majority of tests

* comment & then fix test

* torture the tests into working

* split find and _find_nodes to take different kinds of functions

* redo 'find' on index

* refactor lca_db to use new find

* refactor SBT to use new find

* comment/cleanup

* refactor out common code

* fix up gather

* use 'passes' properly

* attempted cleanup

* minor fixes

* get a start on correct downsampling

* adjust tree downsampling for regular minhashes, too

* remove now-unused search functions in sbtmh

* refactor categorize to use new find

* cleanup and removal

* remove redundant code in lca_db

* remove redundant code in SBT

* add notes

* remove more unused code

* refactor most of the test_sbt tests

* fix one minor issue

* fix jaccard calculation in sbt

* check for compatibility of search fn and query signature

* switch tests over to jaccard similarity, not containment

* fix test

* remove test for unimplemented LCA_Database.find method

* document threshold change; update test

* refuse to run abund signatures

* flatten sigs internally for gather

* reinflate abundances for saving

* fix problem where sbt indices coudl be created with abund signatures

* more

* split flat and abund search

* make ignore_abundance work again for categorize

* turn off best-only, since it triggers on self-hits.

* add test: 'sourmash index' flattens sigs

* add note about something to test

* fix typo; still broken tho

* location is now a property

* move search code into search.py

* remove redundant scaled checking code

* best-only now works properly for two tests

* 'fix' tests by removing v1 and v2 SBT compatibility

* simplify (?) downsampling code

* require keyword args in MinHash.downsample(...)

* fix bug with downsample

* require keyword args in MinHash.downsample(...)

* fix test to use proper downsampling, reverse order to match scaled

* add test for revealed bug

* remove unnecessary comment

* flatten subject MinHash, too

* add testme comment

* clean up sbt find

* clean up lca find

* add IndexSearchResult namedtuple for search and gather results

* add more tests for Index classes

* add tests for subj & query num downsampling

* tests for Index.search_abund

* refactor a bit

* refactor make_jaccard_search_query; start tests

* even more tests

* test collect, best_only

* more search tests

* remove unnec space

* add minor comment

* deal with status == None on SystemExit

* upgrade and simplify categorize

* restore test

* merge

* fix abundance search in SBT for categorize

* code cleanup and refactoring; check for proper error messages

* add explicit test for incompatible num

* refactor MinHash.downsample

* deal with status == None on SystemExit

* fix test

* fix comment mispelling

* properly pass kwargs; fix search_sbt_index

* add simple tests for SBT load and search API

* allow arbitrary kwargs for LCA_DAtabase.find

* add testing of passthru-kwargs

* re-enable test

* add notes to update docstrings

* docstring updates

* fix test

* fix location reporting in prefetch

* fix prefetch location by fixing MultiIndex

* temporary prefetch_gather intervention

* 'gather' only returns best match

* turn prefetch on by default, for now

* better tests for gather --save-unassigned

* remove unused print

* remove unnecessary check-me comment

* clear out docstring

* SBT search doesn't work on v1 and v2 SBTs b/c no min_n_below

* start adding tests

* test some basic prefetch stuff

* update index for prefetch

* add fairly thorough tests

* fix my dumb mistake with gather

* simplify, refactor, fix

* fix remaining tests

* propogate ValueErrors better

* fix tests

* flatten prefetch queries

* fix for genome-grist alpha test

* fix threshold bugarooni

* fix gather/prefetch interactions

* fix sourmash prefetch return value

* minor fixes

* pay proper attention to threshold

* cleanup and refactoring

* remove unnecessary 'scaled'

* minor cleanup

* added LazyLinearLindex and prefetch --linear

* fix abundance problem

* save matches to a directory

* test for saving matches to a directory

* add a flexible progressive signature output class

* add tests for .sig.gz and .zip outputs

* update save_signatures code; add tests; use in gather and search too

* update comment

* cleanup and refactor of SaveSignaturesToLocation code

* docstrings & cleanup

* add 'run' and 'runtmp' test fixtures

* remove unnecessary track_abundance fixture call

* restore original;

* linear and prefetch fixtures + runtmp

* fix use of runtmp

* copy over SaveSignaturesToLocation code from other branch

* docs for sourmash prefetch

* more doc

* minor edits

* Re-implement the actual gather protocol with a cleaner interface. (#1489)

* initial refactor of CounterGather stuff

* refactor into peek and consume

* move next method over to query specific class

* replace gather implementation with new CounterGather

* many more tests for CounterGather

* remove scaled arg from peek

* open-box test for counter internal data structures

* add num query & subj tests

* add repr; add tests; support stdout

* refactor signature saving to use new sourmash_args collection saving

* specify utf-8 encoding for output

* add flexible output to compute/sketch

* add test to trigger rust panic

* test search --save-matches

* add --save-prefetch to sourmash gather

* remove --no-prefetch option :)

* added --save-prefetch functionality

* add back a mostly-functioning --no-prefetch argument :)

* add --no-prefetch back in

* check for JSON in first byte of LCA DB file

* start adding linear tests

* use fixtures to test prefetch and linear more thoroughly

* comments, etc

* upgrade docs for --linear and --prefetch

* 'fix' issue and test

* fix a last test ;)

* Update doc/command-line.md

Co-authored-by: Tessa Pierce Ward <[email protected]>

* Update src/sourmash/cli/sig/rename.py

Co-authored-by: Tessa Pierce Ward <[email protected]>

* Update tests/test_sourmash_args.py

Co-authored-by: Tessa Pierce Ward <[email protected]>

* Update tests/test_sourmash_args.py

Co-authored-by: Tessa Pierce Ward <[email protected]>

* Update tests/test_sourmash_args.py

Co-authored-by: Tessa Pierce Ward <[email protected]>

* Update tests/test_sourmash_args.py

Co-authored-by: Tessa Pierce Ward <[email protected]>

* Update tests/test_sourmash_args.py

Co-authored-by: Tessa Pierce Ward <[email protected]>

* Update doc/command-line.md

Co-authored-by: Tessa Pierce Ward <[email protected]>

* write tests for LazyLinearIndex

* add some basic prefetch tests

* properly test linear!

* add more tests for LazyLinearIndex

* test zipfile bool

* remove unnecessary try/except; comment

* fix signatures() call

* fix --prefetch snafu; doc

* do not overwrite signature even if duplicate md5sum (#1497)

* try adding loc to return values from Index.find

* made use of new IndexSearchResult.find throughout

* adjust note

* provide signatures_with_location on all Index objects

* cleanup and fix

* Update doc/command-line.md

Co-authored-by: Tessa Pierce Ward <[email protected]>

* Update doc/command-line.md

Co-authored-by: Tessa Pierce Ward <[email protected]>

* fix bug around --save-prefetch with multiple databases

* comment/doc minor updates

Co-authored-by: Luiz Irber <[email protected]>
Co-authored-by: Tessa Pierce Ward <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants