feat: add split and merge modules and bump snapshots for latest versions

[chaitanyakasaraneni]

Description of changes

Updates the samplesheetparser modules for the samplesheet-parser 2.5.1 release.

• New modules: samplesheetparser/split and samplesheetparser/merge, following
  the existing module conventions (shared meta, versions topic channel, stub block,
  nf-test with real + stub cases).
• Version bump: samplesheetparser/diff, info, and validate bumped 1.3.0 → 2.5.1
  in environment.yml (BioConda pin) and the BioContainers/Singularity image in main.nf.
  samplesheet-parser=2.5.1 is published on BioConda and BioContainers
  (quay.io/biocontainers/samplesheet-parser:2.5.1--pyhdfd78af_0).
• Snapshots: regenerated against the 2.5.1 container with
  nf-test --update-snapshot --profile docker — 16 tests passing. Only validate
  output md5s changed (validation JSON now emits an informational notes channel);
  diff/info output is byte-identical across versions.

Tested locally with nf-core modules test <module> --profile docker for all five modules.

---

PR checklist

Closes #XXX

• This comment contains a description of changes (with reason).
• If you've fixed a bug or added code that should be tested, add tests!
• If you've added a new tool - have you followed the module conventions in the contribution docs
• If necessary, include test data in your PR.
• Remove all TODO statements.
• Broadcast software version numbers to topic: versions - See version_topics
• 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.
• Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
  • For modules:
    • nf-core modules test <MODULE> --profile docker
    • nf-core modules test <MODULE> --profile singularity
    • nf-core modules test <MODULE> --profile conda
  • For subworkflows:
    • nf-core subworkflows test <SUBWORKFLOW> --profile docker
    • nf-core subworkflows test <SUBWORKFLOW> --profile singularity
    • nf-core subworkflows test <SUBWORKFLOW> --profile conda

[piplus2]

I've used Claude to help me with the review:

1. merge will fail on the most common real-world input

modules/nf-core/samplesheetparser/merge/main.nf:11

tuple val(meta), path(samplesheets)

Per-project sheets are almost always all named SampleSheet.csv. Staging two files with the same basename raises Process ... input file name collision. The sibling diff module already handles this with stageAs: "old/*" / "new/*". The tests pass only because they use two differently-named files (flowcell_samplesheet.v1.csv / flowcell_samplesheet_b.v1.csv).

tuple val(meta), path(samplesheets, stageAs: "input*/*")

Could you also add a test case with two identically-named sheets?

2. split silently converts V1 sheets to V2

modules/nf-core/samplesheetparser/split/main.nf:31-36

The module never passes --to. In cli.py, split's --to defaults to "v2" (the docs table saying "same as input" is inaccurate) and it's applied via SampleSheetSplitter(..., target_version=target). So the "Split V1 sheet by project" test is actually emitting V2 sheets — surprising for anyone splitting a sheet to feed bcl2fastq.

Either expose --to as an input/arg, or state the V2 default explicitly in meta.yml. merge passes --to correctly and is a good model here.

3. by case handling is inconsistent

modules/nf-core/samplesheetparser/split/main.nf:25,32 — by.toLowerCase() is validated, but raw ${by} is passed to the CLI. cli.py checks by not in ("project", "lane") — case-sensitive — so by = 'Project' clears the Groovy guard and then dies with exit 2. Normalise once and reuse:

def by_norm = by?.toLowerCase()
if (!['project', 'lane'].contains(by_norm)) {
    error "by must be 'project' or 'lane', got: ${by}"
}
...
    --by ${by_norm} \\

merge/main.nf:25,30 has the same asymmetry, but it's harmless there since _resolve_version() lowercases — still worth making symmetric. Both also NPE on a null value instead of producing the intended error message, hence the ?. above.

4. merge requires ≥2 files, unguarded

modules/nf-core/samplesheetparser/merge/main.nf:11 — cli.py exits 2 with fewer than two inputs. Since the module already validates target_version in Groovy, a samplesheets.size() >= 2 guard would give a much clearer failure than a bare exit 2 when a channel happens to yield a single sheet.

Smaller:

• meta.yml JSON field lists are incomplete: merge also emits output_path and summary; split also emits source_version and summary.
• documentation: https://illumina-samplesheet.readthedocs.io now 301-redirects to samplesheet-parser.readthedocs.io. Inherited from the existing modules, so optional here — but it's being copied into two new files.

[SPPearce]

You should be able to inside the nf-test rename the input files to make sure they have the same name

[piplus2]

Thanks for the updates! Re-reviewed at 439c910 (again with Claude's help).

4. merge still has no ≥2 guard

cli.py (2.5.1):

if len(files) < 2:
    typer.echo("Error: at least two input files are required.", err=True)
    raise typer.Exit(code=2)

A channel that happens to emit a single sheet dies with a bare exit 2. Since the module already validates target_version in Groovy, a samplesheets.size() >= 2 check is one line and gives a far clearer message.

New: both modules fail the task on a warning

cli.py exits non-zero on warnings, not just on hard errors:

# merge
has_issues = result.has_conflicts or bool(result.warnings)
raise typer.Exit(code=1 if has_issues else 0)

# split
raise typer.Exit(code=1 if result.warnings else 0)

So a merge that emits only a warning (e.g. a read-length mismatch), or a split where some samples have no Sample_Project, writes its output files and then exits 1 — Nextflow kills the task. merge/meta.yml:7 even documents this ("Exits 0 on clean merge, 1 if conflicts or warnings were found"), but merge/main.nf doesn't handle it.

The sibling diff module already hits this and solves it the same way:

    ${old_sheet} ${new_sheet} > ${prefix}.diff.json || true

Suggest the same for merge/main.nf:35 and split/main.nf:43, ideally with a test case that produces a warning so the behaviour is locked in. The current tests don't catch it because the fixtures merge and split cleanly.

If the intent is that conflicts should fail the task, that needs to distinguish exit 1 from exit 2 rather than letting both through — but a bare warning shouldn't kill a run either way.

[piplus2]

just a tiny comment 👍