Replace exit(1) with ModificationNotSupportedError exception in dianncfg.py

[Copilot]

User description

Addresses code review feedback from #59: using exit(1) in library code terminates the entire Python process, preventing callers from handling errors appropriately.

Changes

• Added custom ModificationNotSupportedError exception class
• Replaced all three exit(1) calls with raising the exception:
  • Unrecognized unimod modification
  • Unsupported labeling modifications (TMT, iTRAQ, etc.)
  • Unprocessable modification fallback
• Fixed grammar in error message ("support" → "supports")
• Added diann_config.cfg to .gitignore (test artifact)

Example

from quantmsutils.diann.dianncfg import get_mod, ModificationNotSupportedError

try:
    result = get_mod("UnknownMod (C)", "fixed_mod")
except ModificationNotSupportedError as e:
    # Handle gracefully instead of process termination
    logger.warning(f"Skipping modification: {e}")

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

---

PR Type

Bug fix, Enhancement

---

Description

• Replace three exit(1) calls with ModificationNotSupportedError exception

• Add custom exception class for unsupported modifications

• Fix grammar in error message ("support" → "supports")

• Enable proper error handling instead of process termination

---

Diagram Walkthrough

flowchart LR
  A["exit(1) calls"] -- "replaced with" --> B["ModificationNotSupportedError"]
  C["Error messages"] -- "improved grammar" --> D["'supports' instead of 'support'"]
  B -- "enables" --> E["Graceful error handling"]

Loading

File Walkthrough

Relevant files

Bug fix

dianncfg.py Replace exit calls with custom exception                                  quantmsutils/diann/dianncfg.py Added ModificationNotSupportedError exception class for unsupported modifications Replaced three exit(1) calls with raising the new exception in get_mod() function Fixed grammar in error message: "support" → "supports" Refactored error handling to store messages in variables before logging and raising +16/-7

---

[coderabbitai]

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.}

[qodo-code-review]

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: 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: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Audit logging: The new error paths log messages, but it is unclear whether these constitute auditable, structured logs for critical actions in this context. Referred Code error_msg = ( "Currently only supported unimod modifications for DIA pipeline. Skipped: " + mod ) logging.error(error_msg) raise ModificationNotSupportedError(error_msg) # TODO support DIA multiplex if ( "TMT" in diann_mod_name or "Label:" in diann_mod_name or "iTRAQ" in diann_mod_name or "mTRAQ" in diann_mod_name or "Dimethyl:" in diann_mod_name ): error_msg = ( "quantms DIA-NN workflow only supports LFQ now! Unsupported modifications: " + mod ) logging.error(error_msg) raise ModificationNotSupportedError(error_msg) 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 logs: The added logging uses free-form error strings without structured fields, making it unclear if logs meet structured logging requirements. Referred Code if tag == 0: error_msg = ( "Currently only supported unimod modifications for DIA pipeline. Skipped: " + mod ) logging.error(error_msg) raise ModificationNotSupportedError(error_msg) 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: The function processes mod strings from external sources without explicit validation in the new code, and it's unclear if upstream sanitization exists. Referred Code def get_mod(mod, mod_type): pattern = re.compile(r"\((.*?)\)") tag = 0 diann_mod_accession = None diann_mod_name = None for modification in unimod_database.modifications: if modification.get_name() == mod.split(" ")[0]: diann_mod_accession = modification.get_accession().replace("UNIMOD:", "UniMod:") + "," + str(modification._delta_mono_mass) diann_mod_name = modification.get_name() tag = 1 break if tag == 0: error_msg = ( "Currently only supported unimod modifications for DIA pipeline. Skipped: " + mod ) logging.error(error_msg) raise ModificationNotSupportedError(error_msg) # TODO support DIA multiplex ... (clipped 16 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]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Remove unreachable dead code block Remove the unreachable else block and the associated elif condition, as the condition is always met at this point in the function's logic. quantmsutils/diann/dianncfg.py [94-119] - elif diann_mod_accession is not None: site = re.findall(pattern, " ".join(mod.split(" ")[1:]))[0] - if site == "Protein N-term": - site = "*n" - elif site == "N-term": - site = "n" - elif len(site.split(" ")) >= 2: - pp = " ".join(site.split(" ")[:-1]) - if pp == "Protein N-term": - pp = "*n" - elif pp == "N-term": - pp = "n" - aa = site.split(" ")[-1] - site = pp + aa - if site == "*nM" and diann_mod_name == "Met-loss" and mod_type == "var_mod": - return diann_mod_accession, site - else: - logging.warning("Restricting to certain terminal AAs isn't directly supported. Please see https://github.com/vdemichev/DiaNN/issues/1791") - return diann_mod_accession, site -else: - error_msg = ( - "Currently only supported unimod modifications for DIA pipeline. Skipped: " - + mod - ) - logging.error(error_msg) - raise ModificationNotSupportedError(error_msg) +if site == "Protein N-term": + site = "*n" +elif site == "N-term": + site = "n" +elif len(site.split(" ")) >= 2: + pp = " ".join(site.split(" ")[:-1]) + if pp == "Protein N-term": + pp = "*n" + elif pp == "N-term": + pp = "n" + aa = site.split(" ")[-1] + site = pp + aa + if site == "*nM" and diann_mod_name == "Met-loss" and mod_type == "var_mod": + return diann_mod_accession, site + else: + logging.warning("Restricting to certain terminal AAs isn't directly supported. Please see https://github.com/vdemichev/DiaNN/issues/1791") +return diann_mod_accession, site Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies an unreachable else block due to prior checks, and removing this dead code improves code clarity and maintainability.	Low
More