fix: replace bare NotImplementedError with CLIError for unimplemented test types#230
fix: replace bare NotImplementedError with CLIError for unimplemented test types#230
Conversation
… test types EVAL and SUBMISSION test types raised NotImplementedError with no user-friendly guidance. Now raises CLIError with tracking issue links. Closes #218
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces NotImplementedError with a custom CLIError across the codebase, specifically within the create_default_config and eval functions, and includes GitHub issue links in the error messages for tracking. Corresponding unit tests have been updated to reflect these changes. Feedback was provided to update the docstring of create_default_config to ensure it accurately describes the new exception type being raised.
There was a problem hiding this comment.
Pull request overview
This PR improves CLI/user experience for unimplemented benchmark modes by replacing bare NotImplementedError exceptions with CLIError messages that include guidance and tracking issue links.
Changes:
- Replace
NotImplementedErrorwithCLIErrorforEVAL/SUBMISSIONdefault config generation. - Replace
NotImplementedErrorwithCLIErrorfor theevalCLI command. - Update unit tests to assert
CLIErrorfor unimplemented config templates.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/unit/config/test_yaml_loader.py |
Updates expectations to CLIError when default configs for EVAL/SUBMISSION are requested. |
tests/unit/config/test_schema.py |
Updates schema tests to expect CLIError and to match the updated messages. |
src/inference_endpoint/main.py |
Makes eval command raise CLIError with a tracking issue URL. |
src/inference_endpoint/config/schema.py |
Makes BenchmarkConfig.create_default_config() raise CLIError (with tracking URLs) for EVAL/SUBMISSION. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review comments: update create_default_config docstring from NotImplementedError to CLIError, and fix test_eval.py to expect CLIError.
viraatc
changed the title
EVAL and SUBMISSION test types raised NotImplementedError with no user-friendly guidance. Now raises CLIError with tracking issue links.
Closes #218
What does this PR do?
Type of change
Related issues
Testing
Checklist