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

Changing default confidence threshold for new detectors from 0.9 to 0.75 #212

Merged
merged 4 commits into from
May 28, 2024

Conversation

timmarkhuff
Copy link
Contributor

@timmarkhuff timmarkhuff commented May 28, 2024

This PR changes default confidence threshold for new detectors from 0.9 to 0.75.

The rationale is:

  1. the web app uses 0.75, so we should be consistent
  2. 0.75 is a more reasonable default, and trying to get started with a detector that has a confidence threshold of 0.9 can be painful because there are so many escalations.

I updated public-api.yaml and then ran make generate to update the necessary files, according to the instructions in DEVELOPING.md.

Copy link
Collaborator

@brandon-groundlight brandon-groundlight left a comment

Choose a reason for hiding this comment

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

I forgot, changing the public-api.yaml is the reason we didn't do this sooner. This is fine, but this is the Absolute Last time that anyone should manually edit the public-api.yaml file

Copy link
Member

@robotrapta robotrapta left a comment

Choose a reason for hiding this comment

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

This should bump us up to 0.16

pyproject.toml Outdated
@@ -9,7 +9,7 @@ packages = [
{include = "**/*.py", from = "src"},
]
readme = "README.md"
version = "0.15.2"
version = "0.15.3"
Copy link
Member

Choose a reason for hiding this comment

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

I think this classifies as a "breaking change" functionally speaking - in that client code will not behave the same after the change. So I think that means we should bump to 0.16.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@timmarkhuff
Copy link
Contributor Author

I forgot, changing the public-api.yaml is the reason we didn't do this sooner. This is fine, but this is the Absolute Last time that anyone should manually edit the public-api.yaml file

Okay, at some point you'll need to teach me what the alternative is :)

@timmarkhuff timmarkhuff merged commit 46823fe into main May 28, 2024
8 checks passed
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.

3 participants