Skip to content

🎨 update schema#679

Open
enryH wants to merge 7 commits intodevfrom
upt_schema
Open

🎨 update schema#679
enryH wants to merge 7 commits intodevfrom
upt_schema

Conversation

@enryH
Copy link
Collaborator

@enryH enryH commented Mar 2, 2026 •
edited
Loading

Update the schema.json using nf-core pipelines schema build command. Mainly defaults and type changes.

  • boolean default is implicitly false
  • integers not as strings

PR checklist

  • This comment contains a description of changes (with reason).
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Mar 2, 2026

Review Summary by Qodo

Update nextflow_schema.json with type and default value corrections

✨ Enhancement

Grey Divider

Walkthroughs

Description 
• Remove default false values from boolean schema fields
• Change reference_channel type from string to number with numeric default
• Update pg_level default to float format for consistency
• Align schema with nf-core pipeline standards
Diagram 
flowchart LR
  A["Schema Updates"] --> B["Boolean Fields"]
  A --> C["Type Changes"]
  A --> D["Default Values"]
  B --> B1["Remove default: false"]
  C --> C1["reference_channel: string to number"]
  D --> D1["pg_level: 2 to 2.0"]
Loading

Grey Divider

File Changes

1. nextflow_schema.json ⚙️ Configuration changes +3/-8

Schema type corrections and default value cleanup

• Removed default: false from 3 boolean parameters: onsite_add_decoys,
 onsite_disable_split_by_charge, onsite_compute_all_scores, and force_transfer_learning
• Changed reference_channel type from string to number and updated default from "126" to
 126.0
• Updated pg_level default value from 2 to 2.0 for numeric type consistency
• Maintains all other schema properties including descriptions, help text, and fa_icon values

nextflow_schema.json

Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Mar 2, 2026 •
edited
Loading

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (2) 📎 Requirement gaps (0)

Grey Divider


Action required

1. reference_channel type mismatch ☑ 📘 Rule violation ✓ Correctness
Description 
nextflow_schema.json now defines reference_channel as a number with default 126.0, but
nextflow.config sets it as the string '126'. This can make the schema inaccurate and cause
parameter validation/UI inconsistencies.
Code

nextflow_schema.json[R867-872]

            "reference_channel": {
-                    "type": "string",
+                    "type": "number",
                "description": "The reference channel, e.g. for calculating ratios.",
                "fa_icon": "fas fa-list-ol",
-                    "default": "126"
+                    "default": 126.0
            },
Evidence 
Compliance requires the parameter schema to accurately reflect parameter definitions with
appropriate types/validation. The PR changes reference_channel to a numeric schema type/default,
while the pipeline config still defines the parameter as a string, creating a mismatch.

AGENTS.md
nextflow_schema.json[867-872]
nextflow.config[68-74]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`reference_channel` is defined as `type: number` with default `126.0` in `nextflow_schema.json`, but it is defined as a string (`&amp;amp;amp;amp;#x27;126&amp;amp;amp;amp;#x27;`) in `nextflow.config`. This makes the schema inaccurate and may lead to inconsistent validation/UI behavior.
## Issue Context
`reference_channel` is passed into the IsobaricAnalyzer command via `${params.reference_channel}`, so schema/config should agree on the parameter type and default.
## Fix Focus Areas
- nextflow_schema.json[867-872]
- nextflow.config[68-74]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Remediation recommended

2. pg_level default not integer ☑ 📘 Rule violation ✓ Correctness
Description 
nextflow_schema.json defines pg_level as type: number and sets default: 2.0 while
restricting values to integer-like enum: [0, 1, 2]. This weakens validation (allows non-integers)
and may cause schema/UI inconsistencies.
Code

nextflow_schema.json[R1147-1151]

                "description": "Controls the protein inference mode",
                "fa_icon": "fas fa-list-ol",
                "enum": [0, 1, 2],
-                    "default": 2
+                    "default": 2.0
            },
Evidence 
The compliance rule requires appropriate validation constraints in nextflow_schema.json. Defining
pg_level as a generic number with a float default while the allowed values are integers can
reduce validation clarity and accuracy.

AGENTS.md
nextflow_schema.json[1145-1151]
nextflow.config[200-208]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pg_level` is constrained to `enum: [0, 1, 2]` but is declared as `type: number` with `default: 2.0` in `nextflow_schema.json`. This is less precise than using an integer type/default.
## Issue Context
`nextflow.config` sets `pg_level = 2` and the parameter is used as `--pg-level $params.pg_level` in DIA-NN modules; it is intended to be an integer mode selector.
## Fix Focus Areas
- nextflow_schema.json[1145-1151]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Missing false defaults 🐞 Bug ✓ Correctness
Description 
Several boolean parameters had their explicit default: false removed from the schema while the
pipeline still sets them to false in nextflow.config. This makes schema-generated parameter docs
and schema-based workflow summaries inconsistent/noisy because defaults are no longer declared in
the schema.
Code

nextflow_schema.json[456]

-                    "default": false,
Evidence 
The pipeline generates a workflow summary using paramsSummaryMap with nextflow_schema.json, but
multiple booleans no longer declare default: false in the schema while being set to false in
nextflow.config. This mismatch affects schema-driven reporting/documentation even when users do
not explicitly configure these parameters.

workflows/quantms.nf[178-180]
nextflow_schema.json[453-459]
nextflow_schema.json[494-505]
nextflow_schema.json[652-657]
nextflow_schema.json[1221-1225]
nextflow.config[148-161]
nextflow.config[118-126]
nextflow.config[214-221]
docs/usage.md[5-5]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Several boolean params that default to `false` in `nextflow.config` no longer declare `&amp;amp;amp;amp;quot;default&amp;amp;amp;amp;quot;: false` in the schema. This makes schema-generated parameter docs omit defaults and can cause schema-based workflow summaries (MultiQC workflow summary) to treat these params as divergent from defaults.
### Issue Context
`workflows/quantms.nf` builds the workflow summary with `paramsSummaryMap(... parameters_schema: &amp;amp;amp;amp;quot;nextflow_schema.json&amp;amp;amp;amp;quot;)`, and docs state parameter documentation is generated from the schema.
### Fix Focus Areas
- nextflow_schema.json[453-459]
- nextflow_schema.json[494-505]
- nextflow_schema.json[652-657]
- nextflow_schema.json[1221-1225]
- nextflow.config[148-161]
- nextflow.config[118-126]
- nextflow.config[214-221]
- workflows/quantms.nf[178-180]
- docs/usage.md[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 2, 2026 •
edited
Loading

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch upt_schema

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

qodo-code-review[bot]
qodo-code-review bot reviewed Mar 2, 2026
View reviewed changes
@enryH
Copy link
Collaborator Author

enryH commented Mar 2, 2026

@ypriverol should I try the nf-core template update?

@ypriverol
Copy link
Member

ypriverol commented Mar 2, 2026

yes, sure.

@enryH
Copy link
Collaborator Author

enryH commented Mar 2, 2026 

<<<<<<< upt_schema
                "enable_diann_mztab": {
                    "type": "boolean",
                    "description": "Export the DIA-NN and DIA results to mzTab",
                    "fa_icon": "far fa-check-square"
=======
                "diann_extra_args": {
                    "type": "string",
                    "description": "Extra arguments appended to all DIA-NN steps. Flags incompatible with specific steps are automatically stripped with a warning.",
                    "fa_icon": "fas fa-terminal",
                    "hidden": false,
                    "help_text": "Pass additional DIA-NN command-line arguments that will be appended to all DIA-NN steps (INSILICO_LIBRARY_GENERATION, PRELIMINARY_ANALYSIS, ASSEMBLE_EMPIRICAL_LIBRARY, INDIVIDUAL_ANALYSIS, FINAL_QUANTIFICATION). Flags that conflict with a specific step are automatically stripped with a warning. For step-specific overrides, use custom Nextflow config files with ext.args."
>>>>>>> dev
                }

Was enable_diann_mztab removed or is it just extended?

jpfeuffer
jpfeuffer reviewed Mar 2, 2026
View reviewed changes
jpfeuffer
jpfeuffer requested changes Mar 2, 2026
View reviewed changes
@github-actions
Copy link

github-actions bot commented Mar 2, 2026 •
edited
Loading

nf-core pipelines lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 9276986

+| ✅ 110 tests passed       |+
#| ❔  19 tests were ignored |#
!| ❗   3 tests had warnings |!
Details

❗ Test warnings:

  • pipeline_todos - TODO string in nextflow.config: Specify any additional parameters here
  • pipeline_if_empty_null - ifEmpty(null) found in /home/runner/work/quantms/quantms/subworkflows/local/id/main.nf: _ ch_software_versions = ch_software_versions.mix(PHOSPHO_SCORING.out.versions.ifEmpty(null))
    _
  • pipeline_if_empty_null - ifEmpty(null) found in /home/runner/work/quantms/quantms/subworkflows/local/dda_id/main.nf: _ ch_software_versions = ch_software_versions.mix(PHOSPHO_SCORING.out.versions.ifEmpty(null))
    _

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 3.5.2
  • Run at 2026-03-03 11:42:09

@ypriverol
Copy link
Member

ypriverol commented Mar 3, 2026 

<<<<<<< upt_schema
                "enable_diann_mztab": {
                    "type": "boolean",
                    "description": "Export the DIA-NN and DIA results to mzTab",
                    "fa_icon": "far fa-check-square"
=======
                "diann_extra_args": {
                    "type": "string",
                    "description": "Extra arguments appended to all DIA-NN steps. Flags incompatible with specific steps are automatically stripped with a warning.",
                    "fa_icon": "fas fa-terminal",
                    "hidden": false,
                    "help_text": "Pass additional DIA-NN command-line arguments that will be appended to all DIA-NN steps (INSILICO_LIBRARY_GENERATION, PRELIMINARY_ANALYSIS, ASSEMBLE_EMPIRICAL_LIBRARY, INDIVIDUAL_ANALYSIS, FINAL_QUANTIFICATION). Flags that conflict with a specific step are automatically stripped with a warning. For step-specific overrides, use custom Nextflow config files with ext.args."
>>>>>>> dev
                }

Was enable_diann_mztab removed or is it just extended?

diann to mztab was removed.

@ypriverol ypriverol self-requested a review March 3, 2026 07:30
ypriverol
ypriverol reviewed Mar 3, 2026
View reviewed changes
enryH and others added 2 commits March 3, 2026 10:47
@enryH
🐛 set reference channel to string
cba2229 
- let's hope the next nf-core tools schema builder does not complain
@ypriverol
Merge branch 'dev' into upt_schema
9276986
@ypriverol ypriverol self-requested a review March 3, 2026 11:32
@jpfeuffer
Copy link
Collaborator

jpfeuffer commented Mar 3, 2026

I personally prefer to keep the float values as defaults for a number to show that it can be a float. Maybe the defaults in the config need to be adapted? Or what are we using there?

@ypriverol
Copy link
Member

ypriverol commented Mar 3, 2026

I actually agree with @jpfeuffer I think is good to give the idea that we can have float, but is also true that in most of the cases in the schema; they are all integers must of the time; mass ranges are most of the time passed as integers and also the limit of the intensity.

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

Reviewers

@daichengxin daichengxin daichengxin left review comments

@qodo-code-review qodo-code-review[bot] qodo-code-review[bot] left review comments

@jpfeuffer jpfeuffer jpfeuffer requested changes

@ypriverol ypriverol ypriverol approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@enryH @ypriverol @jpfeuffer @daichengxin