Errr I'm being dumb. If I wanted to use FASTP with sample specific adapters I would have to use multiMap right?

input:
tuple val(meta), path(reads)
path  adapter_fasta

ch_input
    .multiMap { meta, fastq, adapters ->
        fastq: tuple(meta, fastp)
        adapters: tuple(meta, adapters)
    }

The way the module is written now, yes

Safest option IMO is to patch te module locally and pull the adapters into the fastq tuple

Gonna update the module. Wish me luck everyone

Hahahaha, good luck

You're the best @Adam Talbot

shared a GIF

aaaaand the linting failed 😄

looking good

So, I’m all for merging inputs into one tuple, so that’s an auto approve for me. However, I know there’s some people that don’t like that. So I suggest we launch a bigger inquiry to the maintainrs team before merging.

The one and single big tuple is coming, it's my new #C0749PAEU9F

I will stand by you

I'll wait a tiny bit more, but I'll probably make it around summit

Excellent moment to drop that kind of bomb 😄

we always knew Ben was thinking about this a lot

adapters are sample specific so this is already the "correct" approach in the guidelines

Perfect

I guess FASTP is a bit grandfathered in with an old system

So how about we fix the errors of the ancients and bring everything up to modern standards?

I'm gonna change it to be an input channel of

[ meta, fastq, adapters ]

. This is the only clean way of making it work.

It’s for the best

Need Ben's static typing 😰

Oh how I would love that..

shared a GIF

Done

failing all over the place

and finally closed

good job

Stumbled into this thread while trying to figure out what changed after updating the fastp module in a custom pipeline. A few questions/suggestions: 1. Based on the PR and commit messages, I didn't understand that the goal/benefit of merging adapters into a tuple was to allow unique adapters per sample. After reading this thread I do, but that might be worth mentioning somewhere in the PR (or docs?) for people who end up confused like me. 2. Are there any suggestions / examples of how to update code using the previous version that had a single adapter file? From what I can tell, the main pipelines using fastp or its related subworkflows still use the old version of the module. 3. Out of interest, is there an example somewhere of how you are providing unique adapters per samples too? I'm guessing you'd add these as a column in the samplesheet? EDIT: Found a sort of example I think? https://github.com/nf-core/riboseq/pull/126/files#diff-e97ea58b4790b13071a1f72646bb0ca8177c0ccf2441fd1f0242c34e4dbc0900R180 EDIT2: 4. related question: why don't the docs (https://nf-co.re/modules/fastp/) indicate that the first 3 inputs should be combined into a tuple? EDIT3: Getting

ERROR ~ No such variable: task

errors after trying

ch_samplesheet._map_ { _meta_, _reads_ -> _tuple_(meta, reads, params.fastp_adapter_fasta)},

as my new input. Perhaps I'm better off switching back to the older version for now.

I would have tried something with a combine, but a solve is a solve

Thanks for the feedback @Pieter Moris. Sorry, I wasn't clear in the PR description. I'm normally pretty good at writing PR descriptions, but clearly here I focused on the what and not the why and how. A lesson for all of us, I will update the description retrospectively so future users catch it.

Are there any suggestions / examples of how to update code using the previous version that had a single adapter file?

The subworkflows in the PR should be updated? Again, I'll include a migration guide in the PR.

Out of interest, is there an example somewhere of how you are providing unique adapters per samples too? I'm guessing you'd add these as a column in the samplesheet?

Let's add this in broad strokes to the examples as well.

related question: why don't the docs (https://nf-co.re/modules/fastp/) indicate that the first 3 inputs should be combined into a tuple?

Ehhhh not sure why this isn't correct, the meta.yml was updated...bug?

Cheers for the reply, Adam! (and apologies if my initial comments came over a bit harsh through the text medium, that wasn't intended at all 🙂 )

A lesson for all of us, I will update the description retrospectively so future users catch it.

It's a tough balance to strike I think. I wasn't sure if my confusion was due to my unfamiliarity with some of the workings of nf-core (or a foggy brain) or not. But if you have the time, a migration guide just might help someone else in the future!

Ehhhh not sure why this isn't correct, the meta.yml was updated...bug?

I've noticed some oddities in module docs like this in other places too. Again, I wasn't sure if it was just me missing something, but it feels like sometimes parameters are split out into separate inputs, even though they are supplied as tuples?

Updated the PR description now

Brilliant! Much nicer than my temporary solution 🙂