Skip to content

feat: Enhance config validation (Resolves #33) - #42

Open
dyrpsf wants to merge 2 commits into
draeger-lab:devfrom
dyrpsf:feature/issue-33-config-validation
Open

feat: Enhance config validation (Resolves #33)#42
dyrpsf wants to merge 2 commits into
draeger-lab:devfrom
dyrpsf:feature/issue-33-config-validation

Conversation

@dyrpsf

@dyrpsf dyrpsf commented Jul 13, 2026

Copy link
Copy Markdown

Description

This PR introduces a robust, dictionary-based configuration validation system to resolve Issue #33. It centralizes the validation logic into a new config_validator.py utility, replacing the previously verbose YAML traversal[cite: 2].

Key Changes

  • Centralized Schema Validation: Created SchemaField and validate_config to allow developers to define expected configuration structures cleanly.
  • Deprecated Keyword Handling:
    • Replaced the USER -> None fallback with a system that warns the user and automatically applies sensible developer-defined defaults[cite: 2].
    • The _USER_ keyword now throws an explicit ConfigValidationError for missing required parameters[cite: 2].
  • Input Correctness: Added built-in support to check if numerical inputs are within specific ranges (min_val/max_val) and if string parameters match an expected list (allowed_values)[cite: 2].
  • Path Verification: Added is_path and path_must_exist flags to strictly verify that provided file/directory paths actually exist on the system[cite: 2].

Testing

Tested locally to ensure accurate error throwing for missing parameters and out-of-bound variables, as well as successful fallbacks for deprecated keywords.

Resolves draeger-lab#33. Replaces verbose YAML traversal and USER/_USER_ keyword logic with a recursive dictionary schema validator. Adds support for type enforcement, range checking, path validation, and sensible defaults.
@dyrpsf
dyrpsf changed the base branch from main to dev July 16, 2026 11:29
@draeger
draeger requested review from cb-Hades and Copilot July 29, 2026 11:40

Copilot AI left a comment

Copy link
Copy Markdown

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 adds a schema-driven, dictionary-based configuration validation utility intended to centralize and simplify config validation logic, in line with Issue #33.

Changes:

  • Added src/specimen/util/config_validator.py introducing SchemaField, validate_config, and ConfigValidationError.
  • Updated src/specimen/util/__init__.py to expose the new validation utilities at the specimen.util package level.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/specimen/util/config_validator.py New schema-based config validation implementation with type/constraint/path checks and legacy keyword handling.
src/specimen/util/init.py Re-exports new validator symbols from the util package namespace.

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

Comment thread src/specimen/util/config_validator.py Outdated
Comment thread src/specimen/util/config_validator.py
Comment thread src/specimen/util/config_validator.py Outdated
Comment thread src/specimen/util/__init__.py Outdated
Fixes nested USER missing value check logic, cleans up type error formatting, removes unused Callable import, and resolves export ambiguity in __init__.py.
@dyrpsf

dyrpsf commented Aug 1, 2026

Copy link
Copy Markdown
Author

Thanks for the thorough review! I've pushed updates to address all the feedback:

  • Fixed Keyword Logic: Refactored the required field checks so legacy USER keywords are correctly caught alongside _USER_ and handled appropriately without nested logic traps.
  • Cleaner Type Errors: Updated the type validation error message to use type(value).__name__ for readability.
  • Cleaned Imports: Removed the unused Callable import to ensure linting passes.
  • Resolved API Ambiguity: Updated __init__.py to export the config_validator module directly rather than individual symbols.

Let me know if anything else is needed!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants