Skip to content

Comments

Fix contaminant_affix help text to match default value#435

Merged
ypriverol merged 2 commits intodevfrom
copilot/fix-contaminant-affix-default
Nov 26, 2025
Merged

Fix contaminant_affix help text to match default value#435
ypriverol merged 2 commits intodevfrom
copilot/fix-contaminant-affix-default

Conversation

Copy link
Contributor

Copilot AI commented Nov 26, 2025

  • Understand the issue: The contaminant_affix option has default value CONT but help text says CONT_
  • Fix help text to match the actual default value of CONT
  • Verify the fix
  • Run code review and security checks (no issues found)
  • Verify CONT default works across all plugins (MaxQuant, DIA-NN, quantms, mzidentml)
Original prompt

On cli.py:41-41, Looks like the default value for contaminant affix is CONT, but in the help it says CONT_. Can you change it to CONT as default if that is the case.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.


PR Type

Bug fix


Description

  • Fixed help text for --contaminant_affix option

  • Corrected default value documentation from CONT_ to CONT

  • Aligns help text with actual default parameter value


Diagram Walkthrough

flowchart LR
  A["Help text<br/>default: CONT_"] -- "correction" --> B["Help text<br/>default: CONT"]
  C["Actual default<br/>CONT"] -- "now matches" --> B
Loading

File Walkthrough

Relevant files
Bug fix
cli.py
Correct contaminant_affix help text default value               

pmultiqc/cli.py

  • Updated contaminant_affix option help text
  • Changed default value documentation from CONT_ to CONT
  • Ensures help text matches the actual default parameter value
+1/-1     

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 26, 2025

Important

Review skipped

Bot user detected.

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.


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

Co-authored-by: ypriverol <52113+ypriverol@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix default value for contaminant affix in cli.py Fix contaminant_affix help text to match default value Nov 26, 2025
Copilot AI requested a review from ypriverol November 26, 2025 10:26
@ypriverol ypriverol marked this pull request as ready for review November 26, 2025 10:29
@qodo-code-review
Copy link
Contributor

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: 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:
No logging change: The new code only updates CLI help text and does not impact or add any audit logging for
critical actions, which may be acceptable given the scope but cannot be verified from this
diff alone.

Referred Code
    default="CONT",
    help="The contaminant prefix or suffix used or to be used (default: CONT)",
)

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:
No error paths: The change only updates help text for a CLI option and adds no error handling, which is
likely out of scope but cannot confirm robustness from this diff alone.

Referred Code
    default="CONT",
    help="The contaminant prefix or suffix used or to be used (default: CONT)",
)

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 scope: The modification only adjusts CLI help text and does not add or alter input validation;
adequacy of validation for the option cannot be determined from this diff.

Referred Code
    default="CONT",
    help="The contaminant prefix or suffix used or to be used (default: CONT)",
)

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
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Automatically show default value in help

Use the show_default=True parameter in the click.option to automatically display
the default value and remove the hardcoded default from the help string.

pmultiqc/cli.py [39-40]

 default="CONT",
-help="The contaminant prefix or suffix used or to be used (default: CONT)",
+show_default=True,
+help="The contaminant prefix or suffix used or to be used.",
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly proposes using show_default=True to prevent the kind of inconsistency between the default value and the help text that this PR is fixing, thus improving maintainability.

Medium
  • More

@ypriverol
Copy link
Member

@copilot can you make sure that in fact for all the plugins the default value CONT works?

Copy link
Contributor Author

Copilot AI commented Nov 26, 2025

@copilot can you make sure that in fact for all the plugins the default value CONT works?

I verified that the default value CONT works across all plugins:

  • MaxQuant plugin: Uses "potential contaminant" column with "+" values from the MaxQuant file format - doesn't use contaminant_affix parameter.
  • DIA-NN plugin: Basic contaminant handling via a different mechanism.
  • quantms plugin: Uses contaminant_affix with str.contains() for filtering protein accessions containing the affix.
  • mzidentml plugin: Similar approach to quantms.

The CONT default works correctly because:

  1. It matches accessions like CONT_P12345
  2. It also matches CONTAMINANT_* accessions (since they contain CONT)

All 15 existing tests pass. The CLI help now correctly shows (default: CONT) (commit 71f4530).

@ypriverol ypriverol merged commit 9b3f6a0 into dev Nov 26, 2025
3 checks passed
@ypriverol ypriverol deleted the copilot/fix-contaminant-affix-default branch December 5, 2025 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants