Auto-round scores down to align scores & medals #74
Conversation
src/dbt_score/scoring.py
Outdated
| def __post_init__(self) -> None: | ||
| """Auto-round score down to 1 decimal place.""" | ||
| self.value = math.floor(self.value * 10) / 10 |
There was a problem hiding this comment.
Instead of overriding the provided score upon instantiation, I would suggest creating a separate @property, i.e. a derived value. This would allow:
- Keeping the original value
- Keeping things correct when mutated (e.g.
score = Score(9.99, 'foo'); score.value = 5)
WDYT?
There was a problem hiding this comment.
Yep, that's definitely also something that can work.
Would you then suggest we only use the property (e.g. rounded_value) in the plain (human readable) output?
There was a problem hiding this comment.
Yes, and maybe keep the non-rounded in the JSON output? Although we could also use the rounded one in JSON, for consistency purposes 🤔
There was a problem hiding this comment.
Let's keep the non-rounded in the machine-readable outputs 👍
There was a problem hiding this comment.
I've updated the PR to reflect this.
|
|
||
| if len(self._failed_models) > 0: | ||
| print() | ||
| print( | ||
| f"Error: model score too low, fail_any_model_under = " | ||
| f"{self._config.fail_any_model_under}" | ||
| ) | ||
| for model, score in self._failed_models: | ||
| print(f"Model {model.name} scored {round(score.value, 1)}") | ||
| for model, model_score in self._failed_models: |
There was a problem hiding this comment.
We were redefining score here. So whilst here, I've updated this.
| == """🥇 \x1B[1mmodel1\x1B[0m (score: 10.0) | ||
| \x1B[1;32mOK \x1B[0m tests.conftest.rule_severity_low | ||
| \x1B[1;31mERR \x1B[0m tests.conftest.rule_severity_medium: Oh noes | ||
| \x1B[1;33mWARN\x1B[0m (critical) tests.conftest.rule_severity_critical: Error | ||
| == """🥇 \x1b[1mmodel1\x1b[0m (score: 10.0) | ||
| \x1b[1;32mOK \x1b[0m tests.conftest.rule_severity_low | ||
| \x1b[1;31mERR \x1b[0m tests.conftest.rule_severity_medium: Oh noes | ||
| \x1b[1;33mWARN\x1b[0m (critical) tests.conftest.rule_severity_critical: Error |
There was a problem hiding this comment.
Interesting, I know hex is not case sensitive, but curious how this changed 🤔
There was a problem hiding this comment.
Appears it's my local ruff kicking in action.
Even though ruff is configured for this project, it's on 0.2.2 and locally have 0.6.1 running.
Will revert this change.
|
Thanks for fixing it @japborst ! 🙌 |
Fixes #61.
The challenge is that a 9.99 score leads to a silver medal, even though 10.0 is printed as in
HumanReadableFormatterwe're rounding to a single decimal.There are two approaches to this problem
Separately, you could argue this is only a problem for scores near thresholds and not in other instances (e.g. 3.35 might be fine to round to 3.4, but 9.99 should be displayed as 9.9).
To keep things simple, I've added a property on the
Scoremodel and used the rounded-down version in the human-readable formatter.