-
Notifications
You must be signed in to change notification settings - Fork 123
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
Added QIIME2 custom reference database support. #667
Conversation
Release 2.6.0
Release 2.6.1
Release 2.7.0
Release 2.7.1
…stored in either a directory or a tarball.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for that PR!
Looks good to me, see comments below.
I think a proper test file could be created with greengenes85 with files
ampliseq/conf/ref_databases.config
Line 305 in 1067c7c
file = [ "https://data.qiime2.org/2023.7/tutorials/training-feature-classifiers/85_otus.fasta", "https://data.qiime2.org/2023.7/tutorials/training-feature-classifiers/85_otu_taxonomy.txt" ] |
"{BARRNAP={barrnap=0.9}, CUSTOM_DUMPSOFTWAREVERSIONS={python=3.11.4, yaml=6.0}, CUTADAPT_BASIC={cutadapt=3.4}, DADA2_DENOISING={R=4.3.1, dada2=1.28.0}, DADA2_FILTNTRIM={R=4.3.1, dada2=1.28.0}, DADA2_QUALITY1={R=4.3.1, ShortRead=1.58.0, dada2=1.28.0}, DADA2_TAXONOMY={R=4.3.1, dada2=1.28.0}, FASTQC={fastqc=0.12.1}, KRAKEN2_KRAKEN2={kraken2=2.1.2, pigz=2.6}, PHYLOSEQ={R=4.3.0, phyloseq=1.44.0}, RENAME_RAW_DATA_FILES={sed=4.7}, TRUNCLEN={pandas=1.1.5, python=3.9.1}, Workflow={nf-core/ampliseq=2.8.0dev}}" |
… --qiime_ref_tax_custom and --classifier.
Co-authored-by: Daniel Straub <[email protected]>
…en-source-background-assets/ampliseq into qiime2_custom_db Conflicts: subworkflows/local/qiime2_preptax.nf workflows/ampliseq.nf
…to qiime2_custom_db
I've put a test with tarball into the existing reftaxcustom case, and added a qiimecustom that tests with a file pair. I think that's a reasonable balance to test different input patterns.
I've added a |
… files it emits are emitted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Matthew,
looks great. Did you run your newly added tests and test
itself and made sure files look fine? If not I'll have a look before I give my ok here.
Co-authored-by: Daniel Straub <[email protected]>
I ran them manually but also added the new test to the CI pipeline and it looks like it passed. I couldn't figure a good way to snapshot the QIIME taxonomic classification as I guess the algorithm isn't deterministic, so I just checked that the classifier and taxonomy tsv report are produced. Edit: caught one more assertion that was bad, one of the failing tests (doubleprimers) in this round looks spurious, and the test is succeeding on my own end so hopefully succeeds in this rerun. |
Thanks, I have the feeling I should also run a few tests just to make sure, I scheduled some time tomorrow, so I expect to approve the PR then. |
I tested, and found: I think the ideal sequence should be: Sorry that you run into that phyloseq bug, I hope it works now. edit: some points are solved above |
I found the problem and fixed the report. When all tests passed I'll merge it if you do not have any objections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Sorry for no reply, was on leave! Thanks for looking at those last things and the advice along the way. |
Added support for using custom reference databases in QIIME2 taxonomic classification via the
--qiime_ref_tax_custom
flag. This brings QIIME2 taxonomic classification into alignment with Kraken and Dada which allow the same.Testing should probably be added, I could do with some advice on how to make this possible with some reduced database that matches the requirement on what can be passed to the flag (must be a directory or tarball as in the Kraken implementation).
PR checklist
nf-core lint
).nextflow run . -profile test,docker --outdir <OUTDIR>
).CHANGELOG.md
is updated.