Adding ONT read support for ampliseq

[d4straub]

Addresses #741.

Generally, we are running with workflows/ampliseq.nf into nextflow 25.10.4 compilation limits, nextflow 26.04.x seems fine. Essentially, the file gets too large to parse with nextflow 25.10.4. There are a couple of solutions: (1) raise minimal nextflow version, (2) reduce file size by outsourcing code to subworkflows, (3) be more code efficient.

Add path for nanopore data analysis:

• Porechop_ABI - adapter cleaning
• Chopper - quality filtering (default -q 16)
• (not new) Cutadapt - primer cleaning
• Savont - ASVs generaton & chimera removal

With 3x Zymo D6305 & 3x Zymo D6311 the generated sequences are (22 expected):

Nf-core test data now includes ONT data & CI tests were added.

Relevant parameters:

• new --nanopore initiates Savont ASV generation instead of DADA2
• --sample_inference pooled & --sample_inference independent work also with Savont, --sample_inference pseudo remains DADA2-specific
• new --savont_options allows to forward additional options to Savont
• new --skip_* params: --skip_porechop_abi, --skip_chopper

Potential considerations:

• --sample_inference pooled might be desirable over independent for Savont (and imho also for DADA2), consider changing default
• the pipeline has now --pacbio, --iontorrent, --nanopore; consider instead param --sequencing_type pacbio|iontorrent|nanopore|illumina. If we split illumina into illumina_se|illumina_pe to distinguish SE and PE data, we can even omit --single_end, I think. Either make illumina (illumina_pe) default or make it a required param without default?
• Savont is also applicable to PacBio data, potentially allow --sequencing_type pacbio with a new parameter to choose the ASV inferrence method, e.g. --asv_calling dada2|savont
• process MERGE_STATS (that produces file overall_summary.tsv with read numbers per sample) is included now 8 times into ampliseq.nf. Maybe instead of merging always 2 tables, the process could be changed so that at accepts an arbitrary number of tables and merges them all in one process execution, i.e. execute process once instead of many times.

PR checklist

• This comment contains a description of changes (with reason).
• If you've fixed a bug or added code that should be tested, add tests!
• If you've added a new tool - have you followed the pipeline conventions in the contribution docs
• If necessary, also make a PR on the nf-core/ampliseq branch on the nf-core/test-datasets repository.
• Make sure your code lints (nf-core pipelines lint).
• Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
• Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
• Usage Documentation in docs/usage.md is updated.
• Output Documentation in docs/output.md is updated.
• CHANGELOG.md is updated.
• README.md is updated (including new tool citations and authors/contributors).

[erikrikarddaniel]

Addresses #741.

Generally, we are running with workflows/ampliseq.nf into nextflow 25.10.4 compilation limits, nextflow 26.04.x seems fine. Essentially, the file gets too large to parse with nextflow 25.10.4. There are a couple of solutions: (1) raise minimal nextflow version, (2) reduce file size by outsourcing code to subworkflows, (3) be more code efficient.

I suppose 2 and 3 would be ideal, but 1 is of course easiest to implement. If we -- following my suggestion below -- go to version 3.0 of the pipeline, raising the Nextflow version would make some sense.

Add path for nanopore data analysis:

[...]

Potential considerations:

* `--sample_inference pooled` might be desirable over `independent` for Savont (and imho also for DADA2), consider changing default

Fine for me. I think I had performance problems earlier, but should be fine now.

* the pipeline has now `--pacbio`, `--iontorrent`, `--nanopore`; consider instead param `--sequencing_type pacbio|iontorrent|nanopore|illumina`. If we split `illumina` into `illumina_se|illumina_pe` to distinguish SE and PE data, we can even omit `--single_end`, I think. Either make `illumina` (`illumina_pe`) default or make it a required param without default?

I think --sequencing_type would make more sense than individual flags, including the illumina_pe and skip --single_end. I have no strong opinion on whether to have a default or not.

This would imply going to version 3.0 IMO since we're breaking the interface.

* Savont is also applicable to PacBio data, potentially allow `--sequencing_type pacbio` with a new parameter to choose the ASV inferrence method, e.g. `--asv_calling dada2|savont`

I agree with that too. Here, I do think a default for the --asv_calling depending on --sequencing_type would make sense, basically dada2 for all except nanopore.

* process `MERGE_STATS` (that produces file `overall_summary.tsv` with read numbers per sample) is included now _8 times_ into `ampliseq.nf`. Maybe instead of merging always 2 tables, the process could be changed so that at accepts an arbitrary number of tables and merges them all in one process execution, i.e. execute process once instead of many times.

I don't see a problem with including a process many times and would keep it as it is as long as there aren't large performance gains to be had or bugs that could be fixed. "Don't fix if it ain't broken".

PR checklist

[...]

[erikrikarddaniel]

Finally -- great work!

See my comments to your questions.

I'm certainly not requiring any changes for this, but I think having tests also for local modules can be quite helpful. At least for key steps. They can, I believe, build on each other, so one doesn't have to prepare so much test data instead relying on calling the other modules in the test for a downstream module.

[d4straub]

Thanks for the review!
Yes, none of the many local modules have their own tests here. But all are covered by the pipeline tests, afaik.
Also thanks for the comments to my question, do you agree that this is rather for a separate PR? I think that would be cleaner?

[erikrikarddaniel]

Thanks for the review! Yes, none of the many local modules have their own tests here. But all are covered by the pipeline tests, afaik. Also thanks for the comments to my question, do you agree that this is rather for a separate PR? I think that would be cleaner?

Yes, that would be cleaner.

[d4straub]

Great, thanks again!