Skip to content

Format #603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Nov 29, 2025
Merged

Format #603

merged 7 commits into from
Nov 29, 2025

Conversation

@enryH
Copy link
Collaborator

@enryH enryH commented Oct 29, 2025 •
edited by qodo-code-review bot
Loading

User description

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>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).

Changes

  • format files (see if this can be automated)
  • update according to the language server hints (mainly syntax updates). See commit messages for links to references
  • figure out and clean-up channel chaining logic for file staging

PR Type

Enhancement

Description

  • Standardize code formatting and whitespace alignment

  • Simplify channel chaining logic for multiqc input collection

  • Improve readability of function calls with consistent parameter formatting

  • Clean up variable assignments and conditional statement structure

Diagram Walkthrough

flowchart LR
  A["Code Formatting<br/>Whitespace & Alignment"] --> B["Improved Readability"]
  C["Channel Logic<br/>Refactoring"] --> D["Simplified multiqc<br/>Input Collection"]
  B --> E["Cleaner Codebase"]
  D --> E
Loading

File Walkthrough
Relevant files
Formatting
quantms.nf
Code formatting and channel logic refactoring                       

workflows/quantms.nf

  • Standardized include statement formatting with consistent spacing
  • Reformatted function calls to use consistent parameter alignment
  • Simplified channel chaining logic for multiqc file collection using
    .mix() instead of nested .combine() operations
  • Improved conditional statement formatting with proper indentation
  • Cleaned up variable assignments by removing excessive spacing
    alignment
  • Refactored SUMMARY_PIPELINE input construction to use intermediate
    multiqc_inputs variable for clarity
  • Updated string concatenation to use + operator consistently
  • Removed trailing blank lines at end of file
 +87/-58 

enryH added 2 commits October 29, 2025 16:41
@enryH
:format: format main workflow file
f4f415f
@enryH
🎨 clean up collection of pmultiqc inputs
e57eec3 
Can be inspected using dump by setting the flag `-dump-channels`:

in main.nf
multiqc_inputs.dump()

see docs:
https://www.nextflow.io/docs/latest/reference/operator.html#dump
@enryH enryH changed the base branch from master to dev October 29, 2025 16:55
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 29, 2025 •
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.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch format

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.

@bigbio bigbio deleted a comment from github-actions bot Oct 29, 2025
@github-actions
Copy link

github-actions bot commented Oct 29, 2025 •
edited
Loading

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

Posted for pipeline commit c33b46f

+| ✅ 111 tests passed       |+
#| ❔  19 tests were ignored |#
!| ❗   1 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in nextflow.config: Specify any additional parameters here

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 3.5.1
  • Run at 2025-11-29 07:49:28

@ypriverol
Copy link
Member

ypriverol commented Nov 29, 2025

@copilot can you fix the trailing space using prettier in the format branch?

Copilot AI mentioned this pull request Nov 29, 2025
Merged
11 tasks
Copy link

Copilot AI commented Nov 29, 2025

@ypriverol I've opened a new pull request, #618, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 2 commits November 29, 2025 07:38
Copilot finished reviewing on behalf of ypriverol November 29, 2025 09:56
Copilot AI reviewed Nov 29, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reformats the main workflow file to improve code consistency and readability, applying Nextflow language server recommendations. The changes include formatting improvements, syntax modernization, and refactoring of channel chaining logic.

Key changes:

  • Standardized formatting for function calls, spacing, and alignment
  • Modernized syntax (e.g., removing spaces before parentheses, proper if-else block formatting)
  • Refactored channel chaining logic for SUMMARY_PIPELINE inputs, replacing combine() with mix() operations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ypriverol ypriverol marked this pull request as ready for review November 29, 2025 09:59
@qodo-code-review
Copy link

qodo-code-review bot commented Nov 29, 2025

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪
Unsafe termination

Description: The pipeline exits immediately with exit(1, 'No protein database provided') when
params.database is missing, which can abruptly terminate execution without cleanup or
context; prefer error() to fail fast with standardized Nextflow error handling and clearer
messaging.
quantms.nf [88-93]

Referred Code 
if (params.database) {
    ch_db_for_decoy_creation = Channel.from(file(params.database, checkIfExists: true))
}
else {
    exit(1, 'No protein database provided')
}
Ticket Compliance
⚪
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

⚪
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing Auditing: New control-flow around database presence and pipeline branching adds critical actions
(exit on missing DB, subworkflow selection) without any explicit auditing or logging in
the added lines.

Referred Code 
if (params.database) {
    ch_db_for_decoy_creation = Channel.from(file(params.database, checkIfExists: true))
}
else {
    exit(1, 'No protein database provided')
}


CREATE_INPUT_CHANNEL.out.ch_meta_config_iso.mix(
    CREATE_INPUT_CHANNEL.out.ch_meta_config_lfq
).first()
    | combine(ch_db_for_decoy_creation)
    | map { it[-1] }
    | set { ch_db_for_decoy_creation_or_null }

ch_searchengine_in_db = params.add_decoys ? Channel.empty() : Channel.fromPath(params.database)
if (params.add_decoys) {
    GENERATE_DECOY_DATABASE(
        ch_db_for_decoy_creation_or_null
    )
    ch_searchengine_in_db = GENERATE_DECOY_DATABASE.out.db_decoy


 ... (clipped 60 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Exit Handling: The new early exit on missing database and error on duplicate search engines add failure
paths without contextual logging or recovery, which may hinder debugging in production.

Referred Code 
if (params.database) {
    ch_db_for_decoy_creation = Channel.from(file(params.database, checkIfExists: true))
}
else {
    exit(1, 'No protein database provided')
}


CREATE_INPUT_CHANNEL.out.ch_meta_config_iso.mix(
    CREATE_INPUT_CHANNEL.out.ch_meta_config_lfq
).first()
    | combine(ch_db_for_decoy_creation)
    | map { it[-1] }
    | set { ch_db_for_decoy_creation_or_null }

ch_searchengine_in_db = params.add_decoys ? Channel.empty() : Channel.fromPath(params.database)
if (params.add_decoys) {
    GENERATE_DECOY_DATABASE(
        ch_db_for_decoy_creation_or_null
    )
    ch_searchengine_in_db = GENERATE_DECOY_DATABASE.out.db_decoy


 ... (clipped 9 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input Validation: While presence of params.database is checked, other external inputs (e.g.,
params.multiqc_* paths and collected channels) are mixed and combined without visible
validation or sanitization in the added lines.

Referred Code 
ch_multiqc_config = Channel.fromPath("${projectDir}/assets/multiqc_config.yml", checkIfExists: true)
ch_multiqc_custom_config = params.multiqc_config ? Channel.fromPath(params.multiqc_config, checkIfExists: true) : Channel.empty()
ch_multiqc_logo = params.multiqc_logo ? Channel.fromPath(params.multiqc_logo, checkIfExists: true) : Channel.empty()
summary_params = paramsSummaryMap(workflow, parameters_schema: "nextflow_schema.json")
ch_workflow_summary = Channel.value(paramsSummaryMultiqc(summary_params))
ch_multiqc_custom_methods_description = params.multiqc_methods_description
    ? file(params.multiqc_methods_description, checkIfExists: true)
    : file("${projectDir}/assets/methods_description_template.yml", checkIfExists: true)
ch_methods_description = Channel.value(methodsDescriptionText(ch_multiqc_custom_methods_description))
// concatenate multiqc input files
ch_multiqc_files = Channel.empty()
ch_multiqc_files = ch_multiqc_files.mix(ch_multiqc_config)
ch_multiqc_files = ch_multiqc_files.mix(ch_workflow_summary.collectFile(name: 'workflow_summary_mqc.yaml'))
ch_multiqc_files = ch_multiqc_files.mix(FILE_PREPARATION.out.statistics)
ch_multiqc_files = ch_multiqc_files.mix(ch_collated_versions)
ch_multiqc_files = ch_multiqc_files.mix(ch_methods_description.collectFile(name: 'methods_description_mqc.yaml', sort: false))
ch_multiqc_quantms_logo = file("${projectDir}/assets/nf-core-quantms_logo_light.png")

// create cross product of all inputs
multiqc_inputs = CREATE_INPUT_CHANNEL.out.ch_expdesign
    .mix(ch_pipeline_results.ifEmpty([]))


 ... (clipped 10 lines)

Learn more about managing compliance generic rules or creating your own custom rules
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

qodo-code-review bot commented Nov 29, 2025

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix incorrect channel aggregation logic

Replace the incorrect mix().collect() channel aggregation with combine() to
create the correct Cartesian product of inputs for the SUMMARY_PIPELINE process.
The current implementation will likely cause the pipeline to fail.

workflows/quantms.nf [198-204]

 multiqc_inputs = CREATE_INPUT_CHANNEL.out.ch_expdesign
-    .mix(ch_pipeline_results.ifEmpty([]))
-    .mix(ch_multiqc_files.collect())
-    .mix(ch_ids_pmultiqc.collect().ifEmpty([]))
-    .mix(ch_consensus_pmultiqc.collect().ifEmpty([]))
-    .mix(ch_msstats_in.ifEmpty([]))
-    .collect()
+    .combine(ch_pipeline_results.collect().ifEmpty([]))
+    .combine(ch_multiqc_files.collect())
+    .combine(ch_ids_pmultiqc.collect().ifEmpty([]))
+    .combine(ch_consensus_pmultiqc.collect().ifEmpty([]))
+    .combine(ch_msstats_in.collect().ifEmpty([]))
  • Apply / Chat
Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies a critical bug introduced in the PR. The change from combine to mix().collect() alters the data structure passed to SUMMARY_PIPELINE, which would likely cause a runtime failure. The proposed fix restores the correct logic for creating a Cartesian product of the input channels.

High
  • More

@enryH
Copy link
Collaborator Author

enryH commented Nov 29, 2025

@ypriverol I tried to understand the main workflow file quantsms.nf, where I used formatting and then locked at the operators of channels to aggregate files. If you see no errors, feel free to merge, but just be aware that it could result in subtle bugs due to the changed channel logic I might not be aware of (I guess atm we are running tests, but not specifically checking that the outputs are as expected).

@ypriverol ypriverol merged commit e44d594 into dev Nov 29, 2025
42 checks passed
@ypriverol
Copy link
Member

ypriverol commented Nov 29, 2025 •
edited
Loading

We do have tests for all the workflows, and I guess if something was messed up we will see it. However, I did reviewed some of the changes and it looks that mainly was style.

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

Reviewers

Copilot code review Copilot Copilot left review comments

@ypriverol ypriverol ypriverol approved these changes

Assignees

No one assigned

Labels

Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@enryH @ypriverol