https://nf-co.re logo
Join Slack
Powered by
Not sure where to post it but how should we deal w...
# modules
k
Not sure where to post it but how should we deal with 
ch_fasta
and 
ch_fasta_fai
inconsistencies in subworkflows and modules? Pleae have a look here: https://github.com/nf-core/modules/issues/11138
a
I think we have been back and forth on this in nf-core and I'm never sure where exactly it shakes out. Personally I have never really understood why keep files and their associated indexes in separate channels and a lot of that is likely to go away with the new record syntax thankfully. Given that "fat records" are looking like the new trend, it may be best to start fattening up our tuples in preparation (using the combined tuple)
👍 3
k
I see and agree with that design but the current state is not ideal... E.g I adapted:
Copy code
bowtie2/build
bowtie2/align
bwa/index
bwa/mem
since those modules are affected in chipseq. PR tests obviously fails, I naively ignored how many subworkflows use these modules. I don't mind fixing all subworkflows that use the modules above accordingly to handle 
ch_fasta_fai
. But I also don't want to rush it / would like to get something like a common agreement for that change... since the PR will be a bit bigger than initially planned 😄 and might impact quite a few things downstream.
a
Probably this needs some serious thought/debate by the core maintainers. I think @Simon Pearce and @James Fellows Yates have opined on this topic before but I am not sure if there is any current chatter
👍 1
f
<!subteam^S044DU0NG68>;) I vould vote for one channel (eg fasta_fai). Maybe we can show our agreement (👍 ) for this or diasgreement (👎 ) on this message.
👍🏽 1
👍🏻 1
👍 6
m
One tuple is the way
shared a GIF
n
Yeah I agree, Those files are inherently linked to eachother so why split them up 😛
100 rainbow 2
i
I don't always support huge tuples, but in this case, I see no scenario where one would need different combination of fasta+fai, so it should be a single channel
s
Yeah, I have never understood why we had separate channels for indices.
l
One tuple for me too. I might be part of the problem, as I've converted a few modules to use ch_fasta_fai (all sam and bcf tools).
a
I have a vague memory that it used to be a single channel and then they all got changed so that each file could have its own meta - I think possibly it was driven by some multi-genome use-case but I may have hallucinated that
m
If I remember well, it always was multiple input, and only the first had meta, and the. We started to had meta to everything for multiple genomea
party this 1
i
The guidelines are not particularly helpful on this one, perhaps we should think of an update
party this 2
a
Yeah, I assume this will all get reworked with record types in the next nextflow release but I don't know how quickly nf-core plans to move on
party this 1
s
I don't believe record types will fix all these kind of things 😉
j
a
I think there has been some advocating for single input channels (and indeed only one input channel can be a Queue type channel) so I'm pretty sure it will force the hand a bit
😬 1
j
IMO a single 
meta
should represent a single data entity (sample, reference, etc..). All inputs relating to that input entity be in the same tuple. But this is indeed going to be complicated by records!
m
in the end, isn't everything just one tuple under the hood?
paranoid 1
j
There's a single collected set of inputs, but possibly more than one set of metadata...
m
can't all meta data fit in a single meta map?
j
Yes, but then you have a meta map possibly representing multiple entities - need to handle field overlaps etc.
m
you can always split the meta map in multiples in the subworkflows/pipelines
For me modules should always have 1 input, which should be a tuple with at least a meta map and either a boolean or a path
a
Probably nested records makes most sense but I guess we'll see
m
So until we have a decision what do we do?
👍 1
m
In this case, `[meta, fasta, fai]`seems like the best way to go
j
Put indexes with their files, then see how exactly records fall out, play with them, and develop new standards later?
party this 3
m
ok, so 2 meta, 1 for input, 1 for references
m
yeah, it’s the most pragmatic
and a step towards the ideal one tuple
m
@Jonathan Manning just to inform you of the current set of mind for inputs
👍 1
m
I approve
✅ 1
j
OK TL;DR
But history: 1. the separate input channels is a very old specification from right at the beginning of the specs 2. We DO allow very closely related files to go into multi-file tuples: https://docs-v2--nf-core-docs.netlify.app/docs/specifications/components/modules/input-output-options#required-path-channel-inputs 3. As a lot of people (mostly very vocal samtools fanbois, I would like to note 😉 ) wanted this for a long time, and in fact Nextflow-lead-dev Ben actually also suggested having single input channels (like DSL1, funnily enough) for the vast majority of cases, the plan was to EVENTUALLY switch to single input channels as the primary construct to be defined in the specs - however given this would be a VERY LARGE change, we wanted to wait until another major but related change to inputs - which indeed would be something like records to make the change. So basically this as a specification change is 'coming'. We should really have written down point 3 a long time ago in a ADR or something, but we've not got that far in the communtiy yet
I'm on @Jim Downie’s side though ,that I think a lot the people complaining about this are often only handling e.g. a single reference genome which simplifies a lot of this
k
Thanks @James Fellows Yates for the background, any suggestions to resolve the current situation? I think the eventually state is rather fragile now... with samtools and corresponding subworkflows being changed. I would go ahead and change the 
ch_fasta
-> 
ch_fasta_fai
in subworkflows using bwa, bowtie2 or chromap... but I didn't manage to check today how much work this will be in the end...
j
Can you TL:DR qüestió for me?
On mobile now
k
Going ahead and refactor now or wait? I just double checked modules and subworkflows relying on bwa/bowite2/chromap. This will be a massive change… I can start with with one aligner (bowtie2).
j
If it's simply to allow a input channel of fasta + fai (rather than separate), this is allowed so go ahead
👍 2
Open in Slack
PreviousNext
Powered by