Add rgi/bwt #9585

vinisalazar · 2025-12-18T01:30:57Z

Summary of changes:

rgi/bwt: add draft
rgi/bwt: editing input channel

cc @nickp60

- Use template from rgi/main

- Use fastq pair instead of single fasta file

- add fastq extensions to 'pattern'

- Trying to pass linter

- build KMA/BWT index in workdir instead of program executable dir - update test snapshot

- Don't check temp/ directory on snapshot, is different every time - Update snapshot

- provide input.reads as list - don't check tsv snapshot (it's different every time)

- json files are in random order so snapshot differs every time (I think)

vinisalazar · 2025-12-18T06:10:59Z

@nickp60 we might want to adjust the output channels a little bit, looks to me as though these are the main files:

test.allele_mapping_data.txt
test.artifacts_mapping_stats.txt
test.gene_mapping_data.txt
test.overall_mapping_stats.txt
test.reference_mapping_stats.txt

modules/nf-core/rgi/bwt/main.nf

vinisalazar · 2025-12-30T04:40:28Z

@SPPearce thank you for the review, addressed your comments.

- Use '--local' flag as default

nickp60 · 2025-12-31T17:10:40Z

@vinisalazar Thanks for doing all this! I agree with changing the output to specify those 5 outputs; that seems in line with the description in the docs

vinisalazar · 2026-01-05T02:00:04Z

@SPPearce could this be merged as is, and I can have a go at @nickp60's suggestion on a later PR (along with updates to rgi/main, perhaps)?

SPPearce · 2026-01-14T22:04:29Z

Sorry, missed your message last week.
This module looks a bit ugh, but I see it is based on the rgi/main module which has apparently been in nf-core/modules for years. I also see that @jfy133 has been updating that recently, so I'll just ping him to take a look too.
It would be useful for you to swap to using a topic channel for the version: https://nf-co.re/blog/2025/version_topics before we merge it in.

- Code review from nf-core#9585

- Create topics field in meta.yml

vinisalazar · 2026-01-15T06:38:06Z

@nf-core-bot fix linting

github-actions · 2026-01-15T06:38:57Z

Nothing for me to do here! 🤷
This is probably because the linting errors come from nf-core lint and have to be fixed manually (or with nf-core lint --fix).

jfy133

Ok first pass, some general things:

please remove unnecessary whitespace
If you don't want to EMIT the DB_VERSION etc., you can remove them entirely. I seem to remember we exported for downstream usage (e.g. hAMRonization, which may still be useful here!)

jfy133 · 2026-01-15T14:40:14Z

modules/nf-core/rgi/bwt/main.nf

+
+    rgi \\
+        bwt \\
+        ${args} \\


Suggested change

${args} \\

${args2} \\

Sorry, where exactly would this args2 come from?

So the ext.args variables are defined in pipelines' modules.config file. You need one args variable for each command/section of a piped command ¹, so you don't duplicate the same argument for two diffren tools/subcommands

Footnotes

https://nf-co.re/docs/guidelines/components/modules#each-command-must-have-an-args-variable ↩

i.e., in this case you don't use it for anything (even if it has to be there) but it would be defined in the pipeline logic :)

jfy133 · 2026-01-15T14:41:38Z

modules/nf-core/rgi/bwt/main.nf

+    def read_one = reads instanceof List ? reads[0] : reads
+    def read_two = reads instanceof List && reads.size() > 1 ? reads[1] : null


Please use the nf-core convention of using meta.single to decide how to inject single or paired end reads

An example:

modules/modules/nf-core/bbmap/bbnorm/main.nf

Line 25 in 5e8a1c5

input = meta.single_end ? "in=${fastq.join(',')}" : "in=${fastq[0]} in2=${fastq[1]}"

Don't rush on that one, let me discuss with James ;)

modules/nf-core/rgi/bwt/main.nf

modules/nf-core/rgi/bwt/meta.yml

jfy133 · 2026-01-15T14:46:52Z

modules/nf-core/rgi/bwt/tests/main.nf.test.snap

+        "content": [
+            null,
+            [
+                "test.temp.sam.temp.fsa"
+            ],
+            null,
+            null


This looks odd, is it epected all the output to be null?

@nickp60 would you be able to clarify this one? I'm not super familiar with rgi output.

prototaxites

Little thought about arity :)

modules/nf-core/rgi/bwt/main.nf

Co-authored-by: Jim Downie <[email protected]> Co-authored-by: James A. Fellows Yates <[email protected]>

vinisalazar · 2026-01-18T23:34:59Z

@jfy133 @prototaxites thanks heaps for the review! I merged some of the suggestions and left comments in others.

jfy133 · 2026-01-19T08:19:35Z

Responded - linting is failing though!

vinisalazar added 2 commits December 18, 2025 11:24

rgi/bwt: add draft

75f95ec

- Use template from rgi/main

rgi/bwt: editing input channel

42bf5c7

- Use fastq pair instead of single fasta file

vinisalazar marked this pull request as draft December 18, 2025 01:31

vinisalazar added 8 commits December 18, 2025 12:33

rgi/bwt: edit meta.yaml

77ec4ba

- add fastq extensions to 'pattern'

rgi/bwt: update meta.yaml

fe0039c

- Trying to pass linter

rgi/bwt: add test snapshot

6252f12

rgi/bwt: add '--local' flag

738ffc3

- build KMA/BWT index in workdir instead of program executable dir - update test snapshot

rgi/bwt: update snapshot

47300b4

rgi/bwt: fix tests

c95108c

- Don't check temp/ directory on snapshot, is different every time - Update snapshot

rgi/bwt: continue to fix tests

e90b13c

- provide input.reads as list - don't check tsv snapshot (it's different every time)

rgi/bwt: remove json from snapshots

8db0574

- json files are in random order so snapshot differs every time (I think)

vinisalazar marked this pull request as ready for review December 18, 2025 06:08

SPPearce reviewed Dec 23, 2025

View reviewed changes

modules/nf-core/rgi/bwt/main.nf Outdated Show resolved Hide resolved

SPPearce reviewed Dec 23, 2025

View reviewed changes

modules/nf-core/rgi/bwt/main.nf Show resolved Hide resolved

SPPearce reviewed Dec 23, 2025

View reviewed changes

modules/nf-core/rgi/bwt/main.nf Show resolved Hide resolved

Merge branch 'master' into rgi/bwt

fdf2574

rgi/bwt: remove task.ext value

cd2faf4

- Use '--local' flag as default

vinisalazar force-pushed the rgi/bwt branch from d4ff91f to cd2faf4 Compare December 30, 2025 04:41

vinisalazar added 2 commits January 15, 2026 12:29

rgi/bwt: move version to topics output

c167336

- Code review from nf-core#9585

rgi/bwt: update meta.yml

6efbbe3

- Create topics field in meta.yml

Merge branch 'master' into rgi/bwt

668cf72

jfy133 reviewed Jan 15, 2026

View reviewed changes

prototaxites reviewed Jan 15, 2026

View reviewed changes

modules/nf-core/rgi/bwt/main.nf Outdated Show resolved Hide resolved

modules/nf-core/rgi/bwt/main.nf Outdated Show resolved Hide resolved

Apply suggestions from code review

b1f3cae

Co-authored-by: Jim Downie <[email protected]> Co-authored-by: James A. Fellows Yates <[email protected]>

		def read_one = reads instanceof List ? reads[0] : reads
		def read_two = reads instanceof List && reads.size() > 1 ? reads[1] : null

Add rgi/bwt #9585

Are you sure you want to change the base?

Add rgi/bwt #9585

Uh oh!

Conversation

vinisalazar commented Dec 18, 2025

Uh oh!

vinisalazar commented Dec 18, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vinisalazar commented Dec 30, 2025

Uh oh!

nickp60 commented Dec 31, 2025

Uh oh!

vinisalazar commented Jan 5, 2026

Uh oh!

SPPearce commented Jan 14, 2026

Uh oh!

vinisalazar commented Jan 15, 2026

Uh oh!

github-actions bot commented Jan 15, 2026

Uh oh!

jfy133 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prototaxites left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vinisalazar commented Jan 18, 2026

Uh oh!

jfy133 commented Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants