Skip to content

New module: GRIMER#11663

Open
Jonahnki wants to merge 9 commits into
nf-core:masterfrom
Jonahnki:grimer
Open

New module: GRIMER#11663
Jonahnki wants to merge 9 commits into
nf-core:masterfrom
Jonahnki:grimer

Conversation

@Jonahnki
Copy link
Copy Markdown

Closes #11566

Description

Adds a new nf-core module for GRIMER (v1.1.0), a tool that generates interactive HTML dashboards for contamination detection in metagenomics and viromics datasets.

Developed during the May 2026 nf-core x VirJenDB Virus Bioinformatics Hackathon.

Checklist

  • environment.yml matches bioconda package
  • main.nf passes lint (47/47 tests passed)
  • meta.yml complete and schema-valid
  • nf-test dry run passes (2/2 tests discovered)
  • All pre-commit hooks pass

@Jonahnki Jonahnki requested review from a team as code owners May 17, 2026 15:33
@Jonahnki
Copy link
Copy Markdown
Author

The stub test now uses an existing nf-core/test-datasets file to pass CI. I hope to add in a proper GRIMER-specific count table TSV to nf-core/test-datasets in a follow-up post-PR after review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you create this file on-the-fly within the nf.test please.

Comment thread modules/nf-core/grimer/meta.yml Outdated
pattern: "*.tsv"
ontologies:
- edam: http://edamontology.org/format_3475
- meta3:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call this config or something similar

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review @SPPearce! You're definitely right with the meta3 / config naming. I will rename to config since it's a pipeline-level file not tied to individual samples, and restructure as a plain path input without a meta map wrapper

pass [] to omit.
pattern: "*.tsv"
ontologies:
- edam: http://edamontology.org/format_3475
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use meta, meta2, meta3 to mean Nextflow meta maps, not csv file metadata.
Are these files actually sample dependent? Because if so it should be part of the initial tuple.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review and comments, I'll address them

Comment thread .nf-core.yml
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what is going on with this file, but I don't think you should be changing it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was actually modified automatically by nf-core modules lint when it prompted for repository type on first run. I Will revert it to its original state and exclude it from the PR

Comment thread modules/nf-core/grimer/tests/main.nf.test
then {
assertAll(
{ assert process.success },
{ assert process.out.report }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the snapshot of the outputs.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment, I'll replace { assert process.out.report } with { assert snapshot(process.out).match() } and produce another output snapshot

@Jonahnki
Copy link
Copy Markdown
Author

Thanks for the latest comments @SPPearce! I've gone through everything and pushed some fixe

*The metadata input in main.nf was tripping the linter since anything prefixed with meta gets treated as a Groovy map and renamed it to sample_metadata and that cleared it up
*Synced meta.yml to match while I was at it; --fix had silently collapsed the sample_metadata block to {} so I restored the full field descriptions manually. All 51 lint checks are green now
*For the test part, I swaped out the bare assert calls for snapshot(process.out).match() and added a -stub block. Test data is now generated on-the-fly in setup {} so the static tests/testdata/test_count_table.tsv wasn't needed anymore and I've removed it
*.nf-core.yml is also back to upstream state

@Jonahnki
Copy link
Copy Markdown
Author

Managed to fix the two GRIMER-specific CI failures in the latest push:

*Version eval: grimer --version outputs a multi-line ASCII art banner rather than a clean version string, which caused the snapshot mismatch on CI. Changed the eval to grimer --version 2>&1 | tail -1 which reliably extracts just the bare version number from the last line of the output
*meta.yml sync: ran nf-core modules lint --fix to update the version eval string in both the outputs and topics blocks to match the updated main.nf. All 51 lint checks now passing and the stub snapshot has been regenerated to match

The remaining CI failures (SENTIEON, HMMER, WFMASH, SALSA2, KRAKENUNIQ etc.) are infrastructure-level flakes — runner disk full (no space left on device) and conda HTTP 403 errors and unrelated to this PR

@Jonahnki
Copy link
Copy Markdown
Author

The confirm-pass-lint gate failure is downstream of the multiqc and multiqcsav lint failures, which seems like pre-existing issues in those modules unrelated to this PR. The GRIMER module lint passes cleanly. I'm happy to have a reviewer confirm it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

new module: GRIMER

2 participants