Skip to content
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

Expose evaluations and metrics #319

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

brandon-groundlight
Copy link
Collaborator

Makes metrics and evaluations available

Copy link
Contributor

@CoreyEWood CoreyEWood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments about docstrings/return types but functionality-wise this seems good.

Comment on lines +1003 to +1010
"""
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
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?


:param detector: the detector to get the metrics for

:return: The DetectorMetrics object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any worry that this test will be flaky due to evaluation sometimes taking too long?

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