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: adjust documentation to recommend tax over lca for taxonomic analysis #2777

Merged
merged 6 commits into from
Sep 25, 2023

Conversation

ctb
Copy link
Contributor

@ctb ctb commented Sep 24, 2023

This PR adds cautionary notes to the command line docs, and updates the information on classifying signatures to suggest using tax instead of LCA, and even explains why :).

There is more work to be done - we need to add more tutorials, and adjust the language in classifying-signatures around gather and LCA - but this is a nice standalone PR!

Fixes #2562
Fixes #2772
Fixes #2773

Adds information from #2760
Addresses #2535

@ctb
Copy link
Contributor Author

ctb commented Sep 24, 2023

ready for review @bluegenes

@codecov
Copy link

codecov bot commented Sep 24, 2023

Codecov Report

Merging #2777 (a59effc) into latest (256a705) will increase coverage by 6.93%.
The diff coverage is n/a.

❗ Current head a59effc differs from pull request most recent head a103dd3. Consider uploading reports for the commit a103dd3 to get more accurate results

@@            Coverage Diff             @@
##           latest    #2777      +/-   ##
==========================================
+ Coverage   85.91%   92.85%   +6.93%     
==========================================
  Files         130      104      -26     
  Lines       14812    12423    -2389     
  Branches     2621     2621              
==========================================
- Hits        12726    11535    -1191     
+ Misses       1785      587    -1198     
  Partials      301      301              
Flag Coverage Δ
hypothesis-py 25.81% <ø> (-0.01%) ⬇️
python 92.85% <ø> (-0.01%) ⬇️
rust ?

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

see 27 files with indirect coverage changes

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

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.

This looks good and provides some good context for why gather --> tax over lca.

Only thought - do you want to add a sentence on where LCA is still useful? E.g. tracking down individual k-mers/sequences, exploratory work, etc?

@ctb
Copy link
Contributor Author

ctb commented Sep 25, 2023

Only thought - do you want to add a sentence on where LCA is still useful? E.g. tracking down individual k-mers/sequences, exploratory work, etc?

Added in #2777 (review)

(I had originally put in something like that, and then removed as confusing, so your prompt was a nice reality check!)

@ctb ctb merged commit 66bc910 into latest Sep 25, 2023
18 checks passed
@ctb ctb deleted the doc/tax_over_lca branch September 25, 2023 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants