[iraajgangavaram]

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

[github-actions]

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

[iraajgangavaram]

Most of the lines here are the generated conda lock files (over 6,600). The module itself is five small files.

[nictru]

Two things:

1. Please store the h5ad with compression = 'gzip', I recently added this for all modules in scdownstream
2. scdownstream can run multiple doublet detection methods in parallel. Then it takes a boolean column from each of them, sums them up and applies a user-defined threshold. This PR does not contain the boolean column, so it would cause problems with integrating in scdownstream. Maybe you can write an additional csv doublet_calls.csv or something like that, which contains exactly just the boolean column, and is thus compatible with scdownstream. We should have a standard interface for all doublet detection methods, so that they can be used in parallel without an additional unification process afterwards.

[iraajgangavaram]

Thanks, both done. The h5ad is now written with gzip compression. The predictions output is now a CSV with only the boolean doublet column (named after the prefix, one row per cell barcode), which is the layout DOUBLETREMOVAL reads, so it can be combined with the other methods. I moved the full scDblFinder table (class, score and so on) into a separate scores output.

[vagkaratzas]

• Blocking: the test assertions do not follow the sanitizeOutput(process.out) rule, so the CSV outputs are never checked.
• Blocking: dbr and batch_col should go through ext.args, and the template needs to honour task.ext.args; output paths should use ${prefix} rather than globs.
• Minor: implicit it in tests, missing ontologies on the versions entries in meta.yml.

Gates: lint and nf-test taken from CI (both green); nextflow lint run locally, no errors.

Diff 7.96/10 · Overall 7.96/10
nf-core-reviewer 0.5.0 / vagkaratzas-review (rubric v4) · claude-sonnet-5-5

[iraajgangavaram]

Thanks for the review, all points are addressed. dbr and batch_col are gone from the inputs and everything goes through ext.args now (e.g. dbr = 0.08, samples = "batch"), with tests/nextflow.config setting ext.args from params.module_args. Outputs use ${prefix} paths, the versions entries have the YAML ontology, and the closures are named. In the real tests I snapshot the predictions CSV, the versions and adata.yaml rather than the whole sanitizeOutput(process.out), because the h5ad checksum and the float scores can differ between platforms. Happy to switch to the full snapshot if you prefer.

[vagkaratzas]

Thanks for the updates! The ext.args handling, the ${prefix} output names, the named closure parameters and the versions ontologies are all fixed now.

• Blocking: the non-stub tests still don't snapshot sanitizeOutput(process.out). Use unstableKeys for the channels that change between runs (see the inline suggestion).
• Blocking: versions.yml only reports R and scDblFinder. It should also report every other package in environment.yml.
• Minor: one comment just repeats the code, and the predictions description in meta.yml is longer than one line.

Also worth doing: modules on master now start with the two-line nf-core licence header (see e.g. mafft/align/main.nf). Please add it to main.nf.

Diff 8.50/10 · Overall 8.50/10
nf-core-reviewer 0.5.0 / vagkaratzas-review (rubric v4) · claude-opus-5-5

[vagkaratzas]

• All previous requested changes are addressed: the non-stub tests now snapshot sanitizeOutput(process.out, unstableKeys: ['h5ad', 'scores']) with the adata.yaml and scores.size() checks kept as stable replacements, versions.yml (template and stub) reports every package in environment.yml, the versions ontologies are in meta.yml, the predictions description is one line, the redundant comment is gone and the licence header is in main.nf.
• No new findings. Lint and nf-test are green on the head commit, and nextflow lint reports no errors.

Approval should be on its way soon.

Gates: lint and nf-test taken from CI (both green); nextflow lint run locally, no errors (one "variable declared but not used" warning for args, which the template reads).

Diff 10.00/10 · Overall 10.00/10
nf-core-reviewer 0.5.0 / vagkaratzas-review (rubric v4) · claude-sonnet-5-5

[vagkaratzas]

Nice!