Add sintax #577
Merged
Add sintax #577
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
d4straub
reviewed
May 3, 2023
d4straub
reviewed
May 4, 2023
|
Because I solved the merge conflict: please pull your branch before you continue working! |
|
I broke the |
d4straub
reviewed
May 5, 2023
|
I think |
d4straub
reviewed
May 9, 2023
There was a problem hiding this comment.
I think that looks great!
Moving dada2 & sintax taxonomy classification to own subworkflows simplifies ampliseq.nf significantly, I think. But my recommendation to put phylogenetic placement taxonomical assignment into its own subworkflow was a very bad idea, sorry, because that is already a subworkflow.
d4straub
approved these changes
May 10, 2023
There was a problem hiding this comment.
Great simplification of the DADA2 taxonomic workflow, thanks! Thats so much easier to read now I think.
I tested a little (not everything) and all I saw was fine, except two tiny points:
- the sintax taxonomy table had NA while other (DADA2, QIIME2) are blank when no info is available
- Processes SINTAX_TAXONOMY_WF:* and QIIME2_TAXONOMY:QIIME2_CLASSIFY are shown even when not executed, while DADA2_TAXONOMY_WF is not shown when not executed
The first point might be good to address, the second point doesnt matter I think.
On an unrelated note, I recognized that NFCORE_AMPLISEQ:AMPLISEQ:QIIME2_DIVERSITY:QIIME2_TREE is not listed and therefore no QIIME2_DIVERSITY:* is executing with -profile test. That still works with 2.5.0 but seems already broken in dev, so seems unrelated to your PR. I might have broken that in the phylogenetic palcement PR, because I fiddled with the tree info. Will need to investigate at another time.
Thanks for the pointers!
True, fixed it.
This I think is because there is a check if DADA2_TAXONOMY_WF should run, while there is no test for e.g. SINTAX_TAXONOMY_WF:*, instead they are fed an empty input channel and therefore not run. I added a test for sintax, but left the qiime things as they are. There are a lot of qiime modules that are run or not depending on their input channels. |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the SINTAX algorithm as an alternative for taxonomy assignment, and addresses issue #471.
If --sintax_ref_taxonomy is given, SINTAX is used for taxonomical classification in addition to DADA2. The SINTAX taxonomy is used instead of any other classification in downstream steps (qiime, sbdiexport). I also added the option --skip_dada_taxonomy, to skip running DADA2 to get taxonomies.
PR checklist
nf-core lint).nextflow run . -profile test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).