Repository navigation
Clear pipeline-level nf-core lint warnings, bump to 1.2.0dev - #227
Open
ljwharbers wants to merge 6 commits into
Open
ljwharbers wants to merge 6 commits into
ljwharbers wants to merge 6 commits into
Conversation
- manifest.version 1.1.0 -> 1.2.0dev; snapshot version keys follow - move the 13 flat subworkflows/local/*.nf files to <name>/main.nf (includes one level deeper; explicit .nf paths in paired/, tumor_only/ and the tests updated) - skip_modkit moved into skip_options; descriptions for generate_gvcf, autocorrelation, vep_custom, vep_custom_tbi, severus_minsupport and wakhan_chroms - resolve template TODOs: pipeline citation in the methods description, pipeline-specific contribution guidelines, and drop comments whose work was already done - remove lint_log.txt, a lint debug log committed by mistake - RO-Crate regenerated Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| "autocorrelation": { | ||
| "type": "string" | ||
| "type": "string", | ||
| "description": "Pass --acf to fibertools qc to add the nucleosome autocorrelation to the Fiber-seq QC output. Any non-empty value turns it on." |
There was a problem hiding this comment.
This description keeps autocorrelation as a free-form string, but docs/usage.md:307 documents it as a boolean with default false, and it only feeds a truthiness check (params.autocorrelation ? "--acf" : '' in conf/modules.config:83). The new text says "Any non-empty value turns it on", which is wrong in two cases:
--autocorrelation false: Nextflow parses the CLI value to Booleanfalse, so--acfis not added.- A bare
--autocorrelationflag, as the usage.md table suggests, sets the param to Booleantrue. With"type": "string", nf-schema rejects that value.
Since this PR already edits the entry, I'd make the type match the docs: "type": "boolean", "default": false (and autocorrelation = false in nextflow.config), with a description like "Pass --acf to fibertools qc to add nucleosome autocorrelation to the Fiber-seq QC output."
Collaborator
Author
There was a problem hiding this comment.
Fixed in 8f0714a: autocorrelation is now "type": "boolean", "default": false (and autocorrelation = false in nextflow.config), with the suggested description. Checked with nextflow run . -profile test,apptainer -preview: a bare --autocorrelation validates and resolves to true, and --autocorrelation false validates and stays at the default.
| - [ ] Keep the `[ meta, data ]` tuple shape across module and subworkflow boundaries, so sample metadata reaches every downstream step. | ||
| - [ ] Put local subworkflows in `subworkflows/local/<name>/main.nf`. | ||
| - [ ] Add a `CHANGELOG.md` entry with the PR number for every user-facing change. | ||
| - [ ] Tag fast (stub) tests with `small`: pull-request CI runs only tests with that tag. The extended pipeline tests (`clair_only`, `consensus`, `deep_only`, `union`) run only on release or manual dispatch, so regenerate their snapshots whenever an output changes. |
There was a problem hiding this comment.
The list of extended pipeline tests leaves out chm13. tests/chm13.nf.test is tagged extended, not small, so like the others it only runs on release or workflow_dispatch. It's also the only CHM13 coverage, and this PR updates its snapshot too. A contributor who follows this checklist would skip regenerating it. Suggest listing it as well (chm13, clair_only, consensus, deep_only, union), or just saying "all tests tagged extended" so the list can't fall out of date.
Collaborator
Author
There was a problem hiding this comment.
Fixed in 8f0714a: the checklist now says "all tests tagged extended; currently chm13, clair_only, consensus, deep_only and union".
| defaultBranch = 'main' | ||
| nextflowVersion = '!>=25.10.4' | ||
| version = '1.1.0' | ||
| version = '1.2.0dev' |
There was a problem hiding this comment.
manifest.version was bumped by hand, but .nf-core.yml still has template.version: 1.1.0 (line 47). nf-core pipelines bump-version 1.2.0dev updates both. nf-core lint's version_consistency check (run with --release) compares the two, and template syncs read the .nf-core.yml value. Please bump .nf-core.yml to 1.2.0dev as well, or re-run bump-version, so they stay in sync.
Collaborator
Author
There was a problem hiding this comment.
Fixed in 8f0714a: .nf-core.yml template.version is now 1.2.0dev. nf-core pipelines lint --release -k version_consistency reports "Version tags are consistent: manifest.version = 1.2.0dev, nfcore_yml.version = 1.2.0dev". Its only failures are the expected "not numeric" ones for a dev version, which go away at release.
|
Automated review: 3 findings, posted inline.
The subworkflow moves look complete: no references to the old flat For a human reviewer: PR CI runs only |
|
…list - autocorrelation is now a boolean defaulting to false (schema and nextflow.config), matching docs/usage.md. A bare --autocorrelation flag now validates, and --autocorrelation false no longer contradicts the description. - .nf-core.yml template.version 1.1.0 -> 1.2.0dev, matching manifest.version (version_consistency: "Version tags are consistent"). - CONTRIBUTING: refer to all tests tagged `extended` and list chm13. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
Extended-test confirmation (re: the human-reviewer note in the automated review)All pipeline-level tests, including every
Other checks on |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Clear the pipeline-level nf-core lint warnings, bump to 1.2.0dev
Stacked on #224 (template update to 4.2.1). The base is
template_update_4.2.1, so this diff only shows the cleanup; GitHub retargets it todevonce #224 merges.After this, the pipeline-level
nf-core pipelines linttests report 0 warnings (previously 48: the set the lint bot comments on).Changes
manifest.version→1.2.0dev, matching the CHANGELOG heading. The six pipeline snapshots record the pipeline version, so I updated"IntGenomicsLab/lrsomatic"in each; nothing else in them changed.subworkflows/local/<name>/main.nf(local_component_structure, 13 files). Their own includes go one level deeper (../../../modules/...). Explicit.nfpaths inpaired/,tumor_only/,subworkflows/local/tests/*.nf.testand the SigProfiler matrixgenerator test are updated.workflows/lrsomatic.nfis unchanged, because Nextflow resolves'../subworkflows/local/ecdna'to either layout.skip_modkitmoved out of the top-levelpropertiesintoskip_options.generate_gvcf,autocorrelation,vep_custom,vep_custom_tbi,severus_minsupportandwakhan_chromsnow have descriptions, written from how the code uses each parameter.tests/nextflow.config, cramino meta.yml);docs/CONTRIBUTING.mdgets a short pipeline-specific checklist.lint_log.txtremoved: a Nov 2025 nf-core lint debug log committed by mistake, which produced 16 TODO warnings.The new text in the CONTRIBUTING checklist and the methods citation is worth a read.
Not addressed
These are outside the pipeline-level set and outside this PR's scope:
meta.ymlfor the local subworkflows, and newer versions available for some nf-core subworkflows.Testing (Mindwell, apptainer, Nextflow 26.04.3, nf-test 0.9.4)
All run without
--update-snapshot, against the committed snapshots:default,clair_only,consensus,deep_only,union(2 tests),chm13: all PASSEDnf-test test --tag small: 140/140 PASSED, including the six moved-subworkflow testsprek run --all-files: all hooks pass, including Nextflow lint🤖 Generated with Claude Code