Migrate narfmap/hashtable to topic channels by xHaMMaDy · Pull Request #12445 · nf-core/modules

xHaMMaDy · 2026-07-27T18:09:06Z

Description

replace the legacy versions.yml output with a versions_narfmap tuple broadcast to topic: versions
use dragen-os --version 2>&1 to report the NARFMAP version
update the module metadata, nf-tests, and generated snapshots for the structured version output

Generated by Codex

PR checklist

Closes #12319

This comment contains a description of changes (with reason).
Tests cover the changed version output.
Remove all TODO statements.
Broadcast software version numbers to topic: versions.
Follow the naming conventions.
Follow the parameters requirements.
Follow the input/output options guidelines.
Add a resource label.
Use BioConda and BioContainers if possible to fulfil software requirements.

Tests

nf-core modules lint narfmap/hashtable (59 passed)
Docker nf-test (2 passed)
Singularity nf-test (2 passed)
Micromamba nf-test using the Conda environment (2 passed)

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

LouisLeNezet

Hi @xHaMMaDy,
Would you have some time to finish this PR ?

LouisLeNezet · 2026-08-03T12:02:40Z

                { assert snapshot(
-                    process.out.bam,
+                    process.out.bam.collect { meta, bamFile -> [meta, file(bamFile).name, file(bamFile).exists(), file(bamFile).size() > 0] },
                    file(process.out.log[0][1]).readLines().findAll { it.startsWith("decompHash") },


This line should be as a separate assertion outside the snapshot.

I meant

assertAll ( { assert process.success }, { assert file(process.out.log[0][1]).readLines().findAll { it.startsWith("decompHash") } }, { assert snapshot( process.out.bam.collect { meta, bamFile -> [meta, file(bamFile).name, bam(bamFile).getReadsMD5()] }, process.out.findAll { key, val -> key.startsWith("versions") }, ).match() } )

LouisLeNezet

You just need now to update the meta and snapshot

LouisLeNezet · 2026-08-03T13:36:24Z

                { assert snapshot(
-                    process.out.bam,
+                    process.out.bam.collect { meta, bamFile -> [meta, file(bamFile).name, file(bamFile).exists(), file(bamFile).size() > 0] },
                    file(process.out.log[0][1]).readLines().findAll { it.startsWith("decompHash") },


I meant

assertAll ( { assert process.success }, { assert file(process.out.log[0][1]).readLines().findAll { it.startsWith("decompHash") } }, { assert snapshot( process.out.bam.collect { meta, bamFile -> [meta, file(bamFile).name, bam(bamFile).getReadsMD5()] }, process.out.findAll { key, val -> key.startsWith("versions") }, ).match() } )

xHaMMaDy · 2026-08-03T14:15:26Z

Pushed an update for the align test snapshots.

meta.yml already mirrors the simplified Samtools eval from main.nf.

I also tested the exact getReadsMD5() form locally for the segfault regression fixture, but it fails inside HTSJDK because the emitted BAM contains an invalid sequence name:

htsjdk.samtools.SAMException: Sequence name ... doesn't match regex

So for this specific malformed-BAM regression test, I moved the decompHash log check outside the snapshot, kept the BAM existence/non-empty checks outside the snapshot, and snapshot only the BAM meta/name plus versions. The normal BAM tests still use getReadsMD5().

Local checks:

narfmap/align/tests/segfault.nf.test passes with Docker and Nextflow 25.10.2
narfmap/align/tests/main.nf.test --update-snapshot passed and updated the normal BAM snapshots
one full targeted local run hit a transient raw.githubusercontent.com staging timeout, and the isolated retry of that same shard passed

Comment added by Codex

Keep the PR branch linear on current master and preserve the reviewed NARFMAP changes.\n\nGenerated by Codex

Use channel-aware sanitization and stable BAM read-hash fields.\n\nGenerated by Codex

Record the empty-read MD5 produced by the fixture across all profiles.\n\nGenerated by Codex

Snapshot stable BAM names because the fixture emits empty-read BAMs and lint rejects empty MD5 values.\n\nGenerated by Codex

xHaMMaDy · 2026-08-21T23:23:27Z

CI is green after the latest update.

I rebased the branch onto current master and kept the PR diff scoped to the NARFMAP modules. The align snapshots now avoid empty BAM MD5 values, and the hashtable snapshot uses stable output names with channel-aware sanitization.

Comment added by Codex

LouisLeNezet

It seems that the narfmap/align topic migration was already done.
If you want to simplify the nf-test for this module, please use sanitizeOutput with the readsMD5Keys to check for the bam file md5 sum.

https://nf-co.re/nft-utils#sanitizeoutput---sanitize-process-output-to-create-clean-snapshots

Also if the log is unstable please use unstableKeys:["log"]

LouisLeNezet · 2026-08-24T07:01:21Z

    tuple val(meta), path('*.log'), emit: log
    tuple val("${task.process}"), val('narfmap'), eval("dragen-os --version 2>&1"), topic: versions, emit: versions_narfmap
-    tuple val("${task.process}"), val('samtools'), eval("samtools version | sed '1!d;s/.* //'"), topic: versions, emit: versions_samtools
+    tuple val("${task.process}"), val('samtools'), eval("samtools version | sed -En '1s/.* ([0-9]+\\.[0-9]+).*/\\1/p'"), topic: versions, emit: versions_samtools


The previous topic expression was better.

LouisLeNezet · 2026-08-24T07:02:17Z

                { assert snapshot(
-                        file(process.out.hashmap[0][1]).name,
-                        file(process.out.versions[0]).name
+                        sanitizeOutput(process.out, ignoreKeys: ["hashmap"]),


If you use sanitizeOutput, you don't need to collect the file names after.

Also you should use unstableKeys here and not ignoreKeys.

Copilot AI review requested due to automatic review settings July 27, 2026 18:09

Copilot AI reviewed Jul 27, 2026

mashehu requested changes Jul 28, 2026

View reviewed changes

Comment thread modules/nf-core/narfmap/hashtable/tests/main.nf.test Outdated

xHaMMaDy enabled auto-merge July 28, 2026 19:22

xHaMMaDy requested a review from mashehu July 28, 2026 19:23

mashehu requested changes Jul 29, 2026

View reviewed changes

Comment thread modules/nf-core/narfmap/align/tests/main.nf.test Outdated

xHaMMaDy requested a review from mashehu July 29, 2026 12:41

SPPearce reviewed Aug 3, 2026

View reviewed changes

Comment thread modules/nf-core/narfmap/align/meta.yml Outdated

LouisLeNezet reviewed Aug 3, 2026

View reviewed changes

Migrate narfmap modules to topic channels

d8543dd

Keep the PR branch linear on current master and preserve the reviewed NARFMAP changes.\n\nGenerated by Codex

xHaMMaDy force-pushed the narfmap_hashtable_topics_migration branch from 147ac60 to d8543dd Compare August 21, 2026 22:51

xHaMMaDy added 3 commits August 22, 2026 01:56

Fix narfmap snapshot assertions

9207cca

Use channel-aware sanitization and stable BAM read-hash fields.\n\nGenerated by Codex

Update narfmap BAM read snapshots

514eb20

Record the empty-read MD5 produced by the fixture across all profiles.\n\nGenerated by Codex

Avoid empty narfmap BAM checksums

e58b7ec

Snapshot stable BAM names because the fixture emits empty-read BAMs and lint rejects empty MD5 values.\n\nGenerated by Codex

LouisLeNezet requested changes Aug 24, 2026

View reviewed changes

Uh oh!

Conversation

xHaMMaDy commented Jul 27, 2026

Description

PR checklist

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LouisLeNezet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LouisLeNezet Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

LouisLeNezet Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LouisLeNezet left a comment

Choose a reason for hiding this comment

Uh oh!

LouisLeNezet Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

xHaMMaDy commented Aug 3, 2026

Uh oh!

xHaMMaDy commented Aug 21, 2026

Uh oh!

LouisLeNezet left a comment

Choose a reason for hiding this comment

Uh oh!

LouisLeNezet Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

LouisLeNezet Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

LouisLeNezet Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants