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 test to confirm failure when summarizing on empty gather #1560

Merged
merged 20 commits into from
Jun 17, 2021

Conversation

hehouts
Copy link
Contributor

@hehouts hehouts commented May 28, 2021

so far:
added "summarize on empty gather.csv" test
will add a few more tests

adds to PR Ref #1543

@codecov
Copy link

codecov bot commented May 28, 2021

Codecov Report

Merging #1560 (f07b4f1) into add-taxonomy (891d951) will increase coverage by 7.83%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##           add-taxonomy    #1560      +/-   ##
================================================
+ Coverage         81.34%   89.17%   +7.83%     
================================================
  Files               109       82      -27     
  Lines             10749     7041    -3708     
  Branches           1260     1260              
================================================
- Hits               8744     6279    -2465     
+ Misses             1781      538    -1243     
  Partials            224      224              
Flag Coverage Δ
python 89.17% <ø> (ø)
rust ?

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

Impacted Files Coverage Δ
src/core/src/index/mod.rs
src/core/src/wasm.rs
src/core/src/ffi/signature.rs
src/core/src/sketch/minhash.rs
src/core/src/from.rs
src/core/src/index/bigsi.rs
src/core/src/ffi/mod.rs
src/core/src/cmd.rs
src/core/src/sketch/nodegraph.rs
src/core/src/encodings.rs
... and 17 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 891d951...f07b4f1. Read the comment docs.

@bluegenes bluegenes mentioned this pull request Jun 10, 2021
14 tasks
@hehouts hehouts changed the title [WIP] addsummarize on empty gather.csv test [MRG] addsummarize on empty gather.csv test Jun 15, 2021
@hehouts
Copy link
Contributor Author

hehouts commented Jun 15, 2021

@bluegenes left a comment on the test im still working on

@hehouts
Copy link
Contributor Author

hehouts commented Jun 16, 2021

@bluegenes I think this is ready to merge in now; the failed checks are in different tests

tests/test_tax.py Outdated Show resolved Hide resolved
tests/test_tax.py Outdated Show resolved Hide resolved
tests/test_tax.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jun 17, 2021

Codecov Report

Merging #1560 (9a77815) into add-taxonomy (24bea0f) will increase coverage by 7.63%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##           add-taxonomy    #1560      +/-   ##
================================================
+ Coverage         81.37%   89.01%   +7.63%     
================================================
  Files               110       83      -27     
  Lines             10944     7236    -3708     
  Branches           1315     1315              
================================================
- Hits               8906     6441    -2465     
+ Misses             1802      559    -1243     
  Partials            236      236              
Flag Coverage Δ
python 89.01% <ø> (ø)
rust ?

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

Impacted Files Coverage Δ
src/core/src/sketch/hyperloglog/mod.rs
src/core/src/sketch/nodegraph.rs
src/core/src/errors.rs
src/core/src/encodings.rs
src/core/src/ffi/minhash.rs
src/core/src/index/search.rs
src/core/src/sketch/minhash.rs
src/core/src/from.rs
src/core/src/ffi/signature.rs
src/core/src/signature.rs
... and 17 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 24bea0f...9a77815. Read the comment docs.

@hehouts hehouts changed the title [MRG] addsummarize on empty gather.csv test [MRG] add test to confirm failure when summarizing on empty gather Jun 17, 2021
Copy link
Contributor

@bluegenes bluegenes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @hehouts! I'll figure out what error I want to use for that 2nd one!

tests/test_tax.py Outdated Show resolved Hide resolved
@bluegenes bluegenes merged commit bee47fd into add-taxonomy Jun 17, 2021
@bluegenes bluegenes deleted the add-tax-tests branch June 17, 2021 22:52
bluegenes added a commit that referenced this pull request Jun 23, 2021
* provide a kind of ridiculous upgrade to lca index to better deal with identifiers/taxonomy

* update load_taxonomy_assignments to be more flexible and pay attention to CLI

* init structure for taxonomy subcommand

* more init

* syntax and add tax to init

* init tax tests

* working tax summarize command

* fix main

* init tests for new tax_utils

* add ascending taxlist

* init classify cmd

* init tax cli testing

* fix filename

* change to function for classify threshold

* add header

* enable single gather result for summarize; mult for classify

* add util script to take output of tax and format for krona viz (#1559)

* get summarized working for summary and krona output

* init test krona output

* add write_summary function

* get classify working again, both summary and krona output

* test write_classification

* init classify cli tests

* init tests for load_taxonomy_assignments

* enable force for getting past duplicated entries in taxonomy csv

* handle and test missing taxonomy info

* classify: handle, test empty gather results, gather results from csv

* split identifiers by default

* standardize spacing

* comments

* init add tax to docs

* [MRG] add a function to take multiple sourmash tax summarize csvs and output a single "abundance" df (#1562)

* add a function to take multiple sourmash tax summarize csvs and output a single 'abundance' df

* add import dep.

* rework format_tax_to_frac for easier testing and use; add tests

* better name and docstring for agg_sumgather_csvs_by_lineage

* init combine command

* debugging code to help track down SBT duplicates/loss problem

* fix, I think

* remove unnecessary code

* add test for duplicate signatures in SBT creation

* see what happens when you run twice

* add missing signatures, oops

* initial refactoring that passes many tests

* factor filename generation out of actual writing

* refactor and cleanup

* add more sigs to test, add note of concern L)

* fix --append tests, too

* refactor out save_exact in favor if save(..., overwrite=True)

* fix some storage stuff in the tests

* make test less confusing?

* Update src/sourmash/sbt_storage.py

Co-authored-by: Luiz Irber <[email protected]>

* define list_sbts() on base Storage class

* properly record duplicate signature names

* move threshold arg parsing into cli/utils

* init changes for multiquery input

* use namedtuple for summarized gather results

* init update for mult files

* adjust for namedtuple output

* mods for namedtuple

* upd utils

* add --from-file to summarize

* working multifile summarize

* --from-csv to --from-file

* somewhat working classify again

* updated classify

* finish fixing combine test

* make taxonomy_csv required

* cleanup

* more cleanup

* use load_pathlist_from_file

* test check_and_load_gather_csvs

* properly restrict kwargs with *

* allow lineage summary table output from summarize

* require rank for krona, lineage summary output formats

* add test for lineage summary output with format_lineage

* add docstrings

* punt cami to separate PR

* raise ValueError on empty gather results

* move notify

* cleanup

* remove tax combine; add tax label

* verson of load_taxonomy that strictly uses headers

* use new tax fn; enable mult taxonomy inputs

* init tax docs

* add classification status to classify output

* add multi db gather test csv

* fix typo

* whoops, actually fix

* handle accession in lineage csv header

* fix line width

* return available ranks from load_taxonomy_csv

* [MRG] add test to confirm failure when summarizing on empty gather (#1560)

* added summarize on empty gather.csv test

* added empty taxonomy csv test

* fix typo

* Update test_tax.py

* trouble shoot tests

* troubleshooting empty gather test

* fixed, maybe? (#1596)

* trying to pull

* cleaned up test_summarize_empty_gather_tax...

* cleaned test_summarize_empty_gather

* fixed test comments

* removed comment

* Update tests/test_tax.py

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

* Update tests/test_tax.py

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

* updated empty-gather test

Co-authored-by: Tessa Pierce Ward <[email protected]>
Co-authored-by: C. Titus Brown <[email protected]>

* init standardize errs

* add good valueerror for empty lineage csv file

* better catch errs in __main__; test all cmds: empty gather, lineage files

* check available ranks, bad gather headers, empty gather, etc

* emit one (and only one) warning per 100% match

* add all functions to __all__ ...is this desired?

* change cli for better arg parsing; add examples to summarize docs

* add mixed strain classify example and assoc data

* doc formatting

* more doc formatting

* more tiny reformatting

* Apply suggestions from code review

Co-authored-by: C. Titus Brown <[email protected]>

* add usage info for each subcommand; upd docs

* minor cleanup and more informative warning

* add checks for duplicated queries, better output for duplicated filenames; label --> annotate

* better catch/print for valueerrors; pyflakes fixes

* summary --> csv_summary

* upd docs

* two-sample lineage summary example

* rename commands

* upd function names too

* minor doc upds

* use f_unique_to_query

* add output dir; add file output notifications

* add cols to summary outputs; adjust accordingly

* trigger GitHub actions

Co-authored-by: C. Titus Brown <[email protected]>
Co-authored-by: Taylor Reiter <[email protected]>
Co-authored-by: Luiz Irber <[email protected]>
Co-authored-by: Hannah Eve Houts <[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.

4 participants