-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Add support for saving/loading evaluators #40983
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
ralph-msft
commented
May 8, 2025
- Mimics the save functionality from Promptflow
- Adds a way to load the saved evaluators
- Adds an additional test for the new save/load code
- Improves the batch run context tests so that they work correctly in more situations
- More code cleanup
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 support for saving and loading evaluators by introducing new persistence methods as well as complementary tests and minor code cleanups. Key changes include:
- Implementing new save/load functionality for evaluators (in _persist modules) and corresponding tests.
- Refactoring test fixtures and client adapter implementations to account for new BatchClient types and legacy SDK conditions.
- Cleaning up and removing obsolete commented code in several modules.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
sdk/evaluation/azure-ai-evaluation/tests/unittests/test_save_eval.py | Updated skip markers and added a new test for load/run evaluators functionality. |
sdk/evaluation/azure-ai-evaluation/tests/unittests/test_evaluate.py | Removed an unused fixture and updated trace destination test to use newer fixtures. |
sdk/evaluation/azure-ai-evaluation/tests/unittests/test_batch_run_context.py | Introduced new client mock fixtures and updated tests for different client types. |
sdk/evaluation/azure-ai-evaluation/tests/conftest.py | Added a restore_env_vars fixture to support tests modifying environment variables. |
sdk/evaluation/azure-ai-evaluation/azure/ai/evaluation/_legacy/_persist/_save.py | Added new evaluator save/load functions with metadata generation and validation. |
Other modules (client, adapters, check, etc.) | Minor refactoring for improved type annotations, removal of obsolete code, and better dependency checks. |
Comments suppressed due to low confidence (1)
sdk/evaluation/azure-ai-evaluation/tests/unittests/test_save_eval.py:29
- Removing the skip marker on the TestSaveEval class may lead to test failures if the Promptflow dependency is missing. Consider reviewing whether the tests should remain conditionally skipped until the dependency is available.
-@pytest.mark.skipif(MISSING_LEGACY_SDK, reason="This test has a promptflow dependency")
sdk/evaluation/azure-ai-evaluation/tests/unittests/test_batch_run_context.py
Show resolved
Hide resolved
Hi @ralph-msft. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days. |
@ralph-msft please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
Hi @ralph-msft. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing |