Expose evaluations and metrics #319
Conversation
CoreyEWood
left a comment
CoreyEWood
left a comment
There was a problem hiding this comment.
Left some comments about docstrings/return types but functionality-wise this seems good.
| """ | ||
| Get a specific evaluation for a detector | ||
| :param detector: the detector to get the evaluation for | ||
| :param evaluation_id: the evaluation id to get | ||
| :return: The DetectorEvaluation object | ||
| """ |
There was a problem hiding this comment.
I think the docstring here is wrong - it doesn't take an evaluation_id. It also returns a dict, should it be returning a DetectorEvaluation object?
There was a problem hiding this comment.
Thanks for checking the docstrings, I left an older version in after I updated the function. Returning a dict is a little bit of a break from our usual format but this exposes the feature until we better formalize what should be in the evaluation.
src/groundlight/experimental_api.py
Outdated
| :param detector: the detector to get the metrics for | ||
| :return: The DetectorMetrics object |
There was a problem hiding this comment.
Same here about the return type.
| assert metrics["summary"]["unconfident_counts"] is not None | ||
| assert metrics["summary"]["total_iqs"] is not None | ||
|
|
||
| # NOTE: this implicitly tests how quickly the evaluation is made available |
There was a problem hiding this comment.
Is there any worry that this test will be flaky due to evaluation sometimes taking too long?
There was a problem hiding this comment.
possibly, it's something I've got my eye one. If it gets in the way of the CICD pipeline I'll raise the amount of time we give to get an evaluation. But tests were passing on my local tests for the time being
Makes metrics and evaluations available