Skip to content

small changes for release#635

Merged
ypriverol merged 5 commits intobigbio:devfrom
ypriverol:dev
Jan 8, 2026
Merged

small changes for release#635
ypriverol merged 5 commits intobigbio:devfrom
ypriverol:dev

Conversation

@ypriverol
Copy link
Member

@ypriverol ypriverol commented Jan 8, 2026

User description

PR checklist

  • 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 pipeline conventions in the contribution docs
  • If necessary, also make a PR on the bigbio/quantms branch on the nf-core/test-datasets repository.
  • 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>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

PR Type

Enhancement, Documentation


Description

  • Change logging level from info to warn for MS2pip warnings

  • Update CHANGELOG with DIA pipeline mzTab default parameter change

  • Document deprecation of luciphor parameters in favor of onsite

  • Add entries for recent pipeline updates and improvements


Diagram Walkthrough

flowchart LR
  A["Logging Changes"] -->|info to warn| B["MS2pip Warnings"]
  C["CHANGELOG Updates"] -->|Added| D["DIA mzTab Default"]
  C -->|Added| E["Deprecations Section"]
  E -->|luciphor params| F["onsite params"]
Loading

File Walkthrough

Relevant files
Enhancement
main.nf
Upgrade MS2pip warnings to warn level                                       

modules/local/utils/msrescore_features/main.nf

  • Changed log.info to log.warn for MS2pip unit compatibility warnings
  • Improves log severity classification for warning messages
  • Two instances updated in conditional branches
+2/-2     
Documentation
CHANGELOG.md
Document parameter changes and deprecations                           

CHANGELOG.md

  • Added entry for PR some updates related with diann, ptms and pmultiqc small issues #614 documenting DIA pipeline mzTab default
    parameter change
  • Created new "Deprecations" section documenting removal of
    luciphor-specific parameters
  • Documented replacement with unified onsite parameters for PTM
    localization
  • Clarifies migration path for users from luciphor to onsite module
+5/-0     

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 8, 2026

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.


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
Copy link
Contributor

qodo-code-review bot commented Jan 8, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
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: Comprehensive Audit Trails

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

Status: Passed

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

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: Robust Error Handling and Edge Case Management

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

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: Security-First Input Validation and Data Handling

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

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:
Unstructured log message: The newly added log.warn messages are plain-text and may not meet the checklist
requirement for structured logging (e.g., JSON), though this may be constrained by
Nextflow logging conventions.

Referred Code
    log.warn "Warning: MS2pip only supports Da unit. Using default from config!"
    ms2_tolerance = 0.05
} else {
    log.warn "Warning: MS2pip only supports Da unit. Using default from config!"
}

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

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 8, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Assign default value after warning
Suggestion Impact:The commit updated the warning-handling branches so that after logging the warning, `ms2_tolerance` is explicitly set to the configured default (`params.ms2features_tolerance`). While it did not hardcode `0.05` as suggested, it implemented the intent of assigning a default value consistent with the warning message.

code diff:

+        } else if (fragment_unit_lower == 'ppm' || params.ms2features_tolerance_unit == 'ppm') {
             log.warn "Warning: MS2pip only supports Da unit. Using default from config!"
-            ms2_tolerance = 0.05
+            ms2_tolerance = params.ms2features_tolerance
         } else {
-            log.warn "Warning: MS2pip only supports Da unit. Using default from config!"
+            log.warn "Warning: MS2pip only supports Da unit. Fragment mass tolerance unit '${meta['fragmentmasstoleranceunit']}' is not supported. Using default from config! In the future, please use 'Da' or 'ppm'."
+            ms2_tolerance = params.ms2features_tolerance
         }

In the else block, assign the default value 0.05 to ms2_tolerance after logging
the warning to ensure the code's behavior matches the warning message.

modules/local/utils/msrescore_features/main.nf [56-58]

 } else {
     log.warn "Warning: MS2pip only supports Da unit. Using default from config!"
+    ms2_tolerance = 0.05
 }

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies and fixes a bug where a warning message indicates a default value is used, but the code does not actually assign it, potentially leading to incorrect calculations.

High
  • Update

@ypriverol ypriverol merged commit 5b69680 into bigbio:dev Jan 8, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

1 participant