https://nf-co.re logo
Join Slack
Powered by
Are there any objections to adding an aligner opti...
# modules
p
Are there any objections to adding an aligner option to the fastq-screen module so that it can use bwa instead of only the hard-coded bowtie option? I'd be happy to make a PR, but won't bother if there are any reasons why the module is written the way it is. https://github.com/nf-core/modules/blob/master/modules/nf-core/fastqscreen/fastqscreen/main.nf
i
I see no problem if you add proper handling for an empty value, the final discussion will be in the PR anyway
p
Cheers! I think I've got that handled.
I did get stuck on modifying the existing tests though. Whenever I try to run them, nf-test tells me there are no tests to run ... edit: oh, it does work when I clone the entire repo. I was working on a copy of just the installed fastq-screen module in my own pipeline. edit2: scratch that. I'll check the nf-test docs to try and figure out how I can run these tests.
s
The module tests are ignored by default from a pipeline
there is a config that does that
nf-test.config has:
Copy code
// ignore tests coming from the nf-core/modules repo
    ignore 'modules/nf-core/**/*', 'subworkflows/nf-core/**/*'
but fastqscreen was an absolute swine to get sorted in the first place
but feel free to add to it
p
(managed to get the tests working, thanks for the insight though! 🙂 ) And yes, I can see it's messy. I recently modified the module that creates a config and database directory for snpEff though, which is an equally opinionated tool that expects specific directory structures and file names instead of just taking in CLI options pointing to relevant files. So while I've succeeded in adding the aligner options, I now realize there's no simple way to provide a set of bwa indices... Bowtie index module returns a folder, but bwa just a set of only additional index files, without the fasta, which fastqscreen expects. Long story short, not quite there yet.
I'm starting to think it might be easier to supply the config file and the index/fasta dir as separate inputs to the module though. But that will break more things downstream that depend on the current behaviour of the module.
s
I believe that #C06PHJM0Y3D is using the module
p
I've got something that seems to work without modifying the original implementation too much. I ended up creating a separate buildfromindex_bwa module though. And minimap might require yet another one. What I also didn't anticipate is that the fastq-screen biocontainer contains bowtie (it's listed as a dependency), but not bwa nor minimap. So we'd need separate biocontainers for those too 😞
i
For the container issue, can't you just use Seqera Containers?
s
Yeah, should be easy to make a new container with all the dependencies you need
p
Ah drat. Then a bunch of things I've done will be redundant by the time that PR comes out. I'll link to it from mine. As for the container: I'll have a look. I haven't used Seqera containers before, only biocontainer ones, and I wasn't sure how they'd respond to an image that contains 3 different alignment tools.
s
note those PRs are months old 😉
Seqera containers makes it super easy to make an image from multiple bioconda/conda-forge recipes
so just add all the tools you want (including bismark by the looks of it)
I would take those as possible inspiration, but they have had no activity for most of a year
p
note those PRs are months old
What do you mean we're not in January 2025 anymore? 😱
Cheers for the tips and links!
PR is opened, but still work in progress! https://github.com/nf-core/modules/pull/9854
It's more or less done! Just need some suggestions on which of the different approaches I've implemented you prefer.
Quick update for this PR to the fastq screen module: I ended up trying a few different implementations (all of which seem to work in my own pipeline tests), but I could use a second pair of eyes to decide on which one is most readable and maintainable. I'll cross-post this to the #CQY2U5QU9 channel in case anyone else has time to chime in! https://github.com/nf-core/modules/pull/9854#issuecomment-3854672442
Open in Slack
PreviousNext
Powered by