-
Notifications
You must be signed in to change notification settings - Fork 3
[metrics] Add custom thresholds for metrics calculations #22
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
c56adac
to
5a28ec9
Compare
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.
The PR looks ok but I suggest to add some checks to validate the input before doing anything else.
grimoirelab_metrics/cli.py
Outdated
@click.option("--pony-threshold", type=float, show_default=True, help="Pony factor threshold, between 0 and 1", default=0.5) | ||
@click.option( | ||
"--elephant-threshold", type=float, show_default=True, help="Elephant factor threshold, between 0 and 1", default=0.5 | ||
) | ||
@click.option( | ||
"--dev-categories-thresholds", | ||
type=str, | ||
show_default=True, | ||
help="Developer categories thresholds, between 0 and 1", | ||
default="0.8,0.95", | ||
) |
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.
Click has options to check that the values are between 0 and 1. I think it's something the script has to check before doing anything else.
@click.option( | ||
"--dev-categories-thresholds", | ||
type=str, | ||
show_default=True, | ||
help="Developer categories thresholds, between 0 and 1", | ||
default="0.8,0.95", | ||
) |
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.
I think it's better to pass to numbers instead of a string that we have to parse later. If it's not possible to do it, click should parse it and convert it to numbers.
grimoirelab_metrics/cli.py
Outdated
binary_file_pattern: str | None = None, | ||
pony_threshold: float = 0.5, | ||
elephant_threshold: float = 0.5, | ||
dev_categories_thresholds: str = "0.8,0.95", |
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.
This should be a tuple of numbers (maybe a named tuple) and not a string.
b93827a
to
5a6f92e
Compare
This commit allows to define custom thresholds for metrics like Pony factor, Elephant factor and Developer onion analysis. Signed-off-by: Jose Javier Merchante <[email protected]>
5a6f92e
to
ad08120
Compare
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.
LGTM
This PR allows defining custom thresholds for metrics like Pony factor, Elephant factor, and Developer onion analysis.
Wait for #15 to be merged to update the tests