-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Fix null value for summary counts in evaluation result converter #43852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
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 null/NaN safety checks to evaluation summary calculation logic. The changes ensure that null or NaN values in model usage statistics and testing criteria results are properly handled rather than causing errors or corrupting summary data.
Key changes:
- Added
_is_none_or_nan()checks before accumulating token counts and criteria statistics - Refactored inline conditional expressions into explicit variable assignments with null/NaN validation
- Updated test data to reflect proper handling of null values in evaluation results
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
_evaluate.py |
Added null/NaN safety checks for model_name, token counts, and testing criteria statistics |
evaluation_util_convert_old_output_test.jsonl |
Updated test data with additional violence evaluator fields and sample data |
evaluation_util_convert_expected_output.json |
Updated expected output to include new fields and corrected error message percentages |
evaluation_util_convert_error_summary.json |
Updated error summary to reflect partial completion (50% vs 100% failure) |
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
If an SDK is being regenerated based on a new API spec, a link to the pull request containing these API spec changes should be included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines