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: update prefetch/gather output to be clearer #2543

Merged
merged 8 commits into from
Apr 5, 2023

Conversation

ctb
Copy link
Contributor

@ctb ctb commented Apr 5, 2023

Fixes #2539

This PR:

  • add tests for very low threshold (1) on gather
  • Adds output to be clear about --threshold-bp when no matches are found: No matches found for --threshold-bp at ...

@codecov
Copy link

codecov bot commented Apr 5, 2023

Codecov Report

Merging #2543 (a224218) into latest (7e90715) will increase coverage by 7.63%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           latest    #2543      +/-   ##
==========================================
+ Coverage   85.18%   92.82%   +7.63%     
==========================================
  Files         133      104      -29     
  Lines       15163    12380    -2783     
  Branches     2608     2610       +2     
==========================================
- Hits        12917    11492    -1425     
+ Misses       1945      587    -1358     
  Partials      301      301              
Flag Coverage Δ
python 92.82% <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/commands.py 91.40% <100.00%> (+0.05%) ⬆️
src/sourmash/search.py 97.95% <100.00%> (ø)

... and 29 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ctb
Copy link
Contributor Author

ctb commented Apr 5, 2023

Ready for review @sourmash-bio/devs - will need to update to latest, and also running afoul of #2544 in the tests.

@@ -797,7 +797,8 @@ def gather(args):
if prefetch_csvout_fp:
prefetch_csvout_fp.flush()

notify(f"Found {len(save_prefetch)} signatures via prefetch; now doing gather.")
display_bp = format_bp(args.threshold_bp)
notify(f"Prefetch found {len(save_prefetch)} signatures with overlap >= {display_bp}.")
Copy link
Contributor

Choose a reason for hiding this comment

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

should we notify anything about #2318?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think linking this PR into that issue is enough? :) yeah that requires a whole different set of foo!

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.

lgtm!

@ctb ctb enabled auto-merge (squash) April 5, 2023 18:40
@ctb ctb disabled auto-merge April 5, 2023 19:38
@ctb ctb merged commit df201c3 into latest Apr 5, 2023
@ctb ctb deleted the update/prefetch_gather_comms branch April 5, 2023 19:38
@ctb ctb mentioned this pull request Apr 6, 2023
1 task
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.

check gather/prefetch --threshold-bp behavior in a few cases
2 participants