Add rgi/bwt by vinisalazar · Pull Request #9585 · nf-core/modules

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

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 #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

+
+    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

+    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 ;)

I think the general consensus was to use meta.single_end for this as James said.

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

+        "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 :)

Co-authored-by: Jim Downie <19718667+prototaxites@users.noreply.github.com> Co-authored-by: James A. Fellows Yates <jfy133@gmail.com>

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!

- capture topics appropriately in meta.yml

vinisalazar · 2026-01-20T03:44:41Z

@jfy133 @SPPearce I've tried a bunch of things and managed to bring the number of errors in the lint down to 1, but can't figure that one out, am out of my depth here, please help 🙏🏼

SPPearce · 2026-02-03T06:06:35Z

Hi @vinisalazar,
Sorry, I missed the last message.
If you still want help, we can get this finished off and merged, it is almost there.

vinisalazar · 2026-02-03T23:01:48Z

Thanks @SPPearce, I accidentally deleted this while cleaning some old repos from my profile, but created the new PR referenced above.

* rgi/bwt: add draft - Use template from rgi/main * rgi/bwt: editing input channel - Use fastq pair instead of single fasta file * rgi/bwt: edit meta.yaml - add fastq extensions to 'pattern' * rgi/bwt: update meta.yaml - Trying to pass linter * rgi/bwt: add test snapshot * rgi/bwt: add '--local' flag - build KMA/BWT index in workdir instead of program executable dir - update test snapshot * rgi/bwt: update snapshot * rgi/bwt: fix tests - Don't check temp/ directory on snapshot, is different every time - Update snapshot * rgi/bwt: continue to fix tests - provide input.reads as list - don't check tsv snapshot (it's different every time) * rgi/bwt: remove json from snapshots - json files are in random order so snapshot differs every time (I think) * rgi/bwt: remove task.ext value - Use '--local' flag as default * rgi/bwt: move version to topics output - Code review from #9585 * rgi/bwt: update meta.yml - Create topics field in meta.yml * Apply suggestions from code review Co-authored-by: Jim Downie <19718667+prototaxites@users.noreply.github.com> Co-authored-by: James A. Fellows Yates <jfy133@gmail.com> * rgi/bwt: fix tests and linting - capture topics appropriately in meta.yml * rgi/bwt: formatting * rgi/bwt: formatting * rgi/bwt: formatting * rgi/bwt: formatting - Remove whitespace * rgi/bwt: update snapshot - Fix escape character in versions output * rgi/bwt: fix module test dataset path Code review for #9873 Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com> * rgi/bwt: apply suggestions from code review (#9873) Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com> * rgi/bwt: use file in test-datasets repo - Closes #9339 * Apply suggestions from code review Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com> * Update modules/nf-core/rgi/bwt/main.nf Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com> --------- Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com> Co-authored-by: Jim Downie <19718667+prototaxites@users.noreply.github.com> Co-authored-by: James A. Fellows Yates <jfy133@gmail.com> Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>

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

Comment thread modules/nf-core/rgi/bwt/main.nf Outdated

SPPearce reviewed Dec 23, 2025

View reviewed changes

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

SPPearce reviewed Dec 23, 2025

View reviewed changes

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

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 #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

Comment thread modules/nf-core/rgi/bwt/main.nf Outdated

Comment thread modules/nf-core/rgi/bwt/main.nf Outdated

Apply suggestions from code review

b1f3cae

Co-authored-by: Jim Downie <19718667+prototaxites@users.noreply.github.com> Co-authored-by: James A. Fellows Yates <jfy133@gmail.com>

rgi/bwt: fix tests and linting

8903429

- capture topics appropriately in meta.yml

vinisalazar force-pushed the rgi/bwt branch from 6af14f3 to 8903429 Compare January 20, 2026 03:17

rgi/bwt: formatting

a63bb29

vinisalazar force-pushed the rgi/bwt branch from 637170b to a63bb29 Compare January 20, 2026 03:30

vinisalazar added 2 commits January 20, 2026 14:31

rgi/bwt: formatting

6647bb8

rgi/bwt: formatting

60c83c3

vinisalazar closed this by deleting the head repository Feb 3, 2026

vinisalazar mentioned this pull request Feb 3, 2026

Add rgi/bwt #9873

Merged

17 tasks

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

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!

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!

vinisalazar commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SPPearce commented Feb 3, 2026

Uh oh!

vinisalazar commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vinisalazar commented Jan 20, 2026 •

edited

Loading