From ae781cd62dc7830a401da088ace0ae101385bcf2 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Fri, 24 May 2024 16:06:39 +0200 Subject: [PATCH 01/21] Add medals --- src/dbt_score/config.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/dbt_score/config.py b/src/dbt_score/config.py index a7d1fc5..275936d 100644 --- a/src/dbt_score/config.py +++ b/src/dbt_score/config.py @@ -16,13 +16,22 @@ class Config: """Configuration for dbt-score.""" _main_section: Final[str] = "tool.dbt-score" - _options: Final[list[str]] = ["rule_namespaces", "disabled_rules"] + _options: Final[list[str]] = [ + "rule_namespaces", + "disabled_rules", + "bronze_medal_threshold", + "silver_medal_threshold", + "gold_medal_threshold", + ] _rules_section: Final[str] = f"{_main_section}.rules" def __init__(self) -> None: """Initialize the Config object.""" self.rule_namespaces: list[str] = ["dbt_score.rules", "dbt_score_rules"] self.disabled_rules: list[str] = [] + self.bronze_medal_threshold: float = 7.0 + self.silver_medal_threshold: float = 8.0 + self.gold_medal_threshold: float = 9.0 self.rules_config: dict[str, RuleConfig] = {} self.config_file: Path | None = None From baf731ceab0a5758fb47341ae4b27590a03922f6 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Mon, 27 May 2024 11:49:01 +0200 Subject: [PATCH 02/21] WIP Inital version of medals --- src/dbt_score/config.py | 15 ++++++++++++ src/dbt_score/evaluation.py | 16 +++++++------ src/dbt_score/formatters/__init__.py | 4 ++-- .../formatters/human_readable_formatter.py | 8 +++---- src/dbt_score/lint.py | 2 +- src/dbt_score/scoring.py | 24 +++++++++++++++++-- 6 files changed, 53 insertions(+), 16 deletions(-) diff --git a/src/dbt_score/config.py b/src/dbt_score/config.py index 275936d..b061281 100644 --- a/src/dbt_score/config.py +++ b/src/dbt_score/config.py @@ -79,6 +79,21 @@ def load(self) -> None: config_file = self.get_config_file(Path.cwd()) if config_file: self._load_toml_file(str(config_file)) + self.validate() + + + def validate(self) -> None: + """Validate the config.""" + if self.bronze_medal_threshold >= self.silver_medal_threshold: + raise ValueError( + "bronze_medal_threshold must be lower than silver_medal_threshold" + ) + if self.silver_medal_threshold >= self.gold_medal_threshold: + raise ValueError( + "silver_medal_threshold must be lower than gold_medal_threshold" + ) + + def overload(self, values: dict[str, Any]) -> None: """Overload config with additional values.""" diff --git a/src/dbt_score/evaluation.py b/src/dbt_score/evaluation.py index 284f1db..f79b629 100644 --- a/src/dbt_score/evaluation.py +++ b/src/dbt_score/evaluation.py @@ -21,11 +21,11 @@ class Evaluation: """Evaluate a set of rules on a set of nodes.""" def __init__( - self, - rule_registry: RuleRegistry, - manifest_loader: ManifestLoader, - formatter: Formatter, - scorer: Scorer, + self, + rule_registry: RuleRegistry, + manifest_loader: ManifestLoader, + formatter: Formatter, + scorer: Scorer, ) -> None: """Create an Evaluation object. @@ -64,12 +64,14 @@ def evaluate(self) -> None: self.results[model][rule.__class__] = result self.scores[model] = self._scorer.score_model(self.results[model]) + model_medal = self._scorer.award_medal(self.scores[model]) self._formatter.model_evaluated( - model, self.results[model], self.scores[model] + model, self.results[model], self.scores[model], model_medal ) # Compute score for project self.project_score = self._scorer.score_aggregate_models( list(self.scores.values()) ) - self._formatter.project_evaluated(self.project_score) + project_medal = self._scorer.award_medal(self.project_score) + self._formatter.project_evaluated(self.project_score, project_medal) diff --git a/src/dbt_score/formatters/__init__.py b/src/dbt_score/formatters/__init__.py index 1734b33..023b487 100644 --- a/src/dbt_score/formatters/__init__.py +++ b/src/dbt_score/formatters/__init__.py @@ -19,12 +19,12 @@ def __init__(self, manifest_loader: ManifestLoader): @abstractmethod def model_evaluated( - self, model: Model, results: ModelResultsType, score: float + self, model: Model, results: ModelResultsType, score: float, medal: str ) -> None: """Callback when a model has been evaluated.""" raise NotImplementedError @abstractmethod - def project_evaluated(self, score: float) -> None: + def project_evaluated(self, score: float, medal: str) -> None: """Callback when a project has been evaluated.""" raise NotImplementedError diff --git a/src/dbt_score/formatters/human_readable_formatter.py b/src/dbt_score/formatters/human_readable_formatter.py index dccf086..7689668 100644 --- a/src/dbt_score/formatters/human_readable_formatter.py +++ b/src/dbt_score/formatters/human_readable_formatter.py @@ -21,7 +21,7 @@ def bold(text: str) -> str: return f"\033[1m{text}\033[0m" def model_evaluated( - self, model: Model, results: ModelResultsType, score: float + self, model: Model, results: ModelResultsType, score: float, medal: str ) -> None: """Callback when a model has been evaluated.""" print(f"Model {self.bold(model.name)}") @@ -35,9 +35,9 @@ def model_evaluated( ) else: print(f"{self.indent}{self.label_error} {rule.source()}: {result!s}") - print(f"Score: {self.bold(str(round(score, 1)))}") + print(f"Score: {self.bold(str(round(score, 1)))} {medal}") print() - def project_evaluated(self, score: float) -> None: + def project_evaluated(self, score: float, medal: str) -> None: """Callback when a project has been evaluated.""" - print(f"Project score: {self.bold(str(round(score, 1)))}") + print(f"Project score: {self.bold(str(round(score, 1)))} {medal}") diff --git a/src/dbt_score/lint.py b/src/dbt_score/lint.py index ff6ca5a..78e6c04 100644 --- a/src/dbt_score/lint.py +++ b/src/dbt_score/lint.py @@ -30,7 +30,7 @@ def lint_dbt_project( formatters = {"plain": HumanReadableFormatter, "manifest": ManifestFormatter} formatter = formatters[format](manifest_loader=manifest_loader) - scorer = Scorer() + scorer = Scorer(config) evaluation = Evaluation( rule_registry=rule_registry, diff --git a/src/dbt_score/scoring.py b/src/dbt_score/scoring.py index 12fa2f1..e176645 100644 --- a/src/dbt_score/scoring.py +++ b/src/dbt_score/scoring.py @@ -4,6 +4,8 @@ import typing +from dbt_score.config import Config + if typing.TYPE_CHECKING: from dbt_score.evaluation import ModelResultsType from dbt_score.rule import RuleViolation, Severity @@ -22,14 +24,20 @@ class Scorer: min_score = 0.0 max_score = 10.0 + def __init__(self, config: Config) -> None: + """Create a Scorer object.""" + self.bronze_medal_threshold = config.bronze_medal_threshold + self.silver_medal_threshold = config.silver_medal_threshold + self.gold_medal_threshold = config.gold_medal_threshold + def score_model(self, model_results: ModelResultsType) -> float: """Compute the score of a given model.""" if len(model_results) == 0: # No rule? No problem return self.max_score if any( - rule.severity == Severity.CRITICAL and isinstance(result, RuleViolation) - for rule, result in model_results.items() + rule.severity == Severity.CRITICAL and isinstance(result, RuleViolation) + for rule, result in model_results.items() ): # If there's a CRITICAL violation, the score is 0 return self.min_score @@ -57,3 +65,15 @@ def score_aggregate_models(self, scores: list[float]) -> float: if len(scores) == 0: return self.max_score return sum(scores) / len(scores) + + def award_medal(self, score: float) -> str: + """Award a medal based on a score.""" + rounded_score = round(score, 1) + if rounded_score >= self.gold_medal_threshold: + return "🥇" + elif rounded_score >= self.silver_medal_threshold: + return "🥈" + elif rounded_score >= self.bronze_medal_threshold: + return "🥉" + else: + return "" From 440bc4dc4e5cd49b6c604ebf9ced73b48626d62a Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Mon, 27 May 2024 14:53:09 +0200 Subject: [PATCH 03/21] Fix and add tests --- src/dbt_score/config.py | 3 -- src/dbt_score/evaluation.py | 10 ++-- .../formatters/manifest_formatter.py | 16 +++--- src/dbt_score/scoring.py | 28 +++++----- .../test_human_readable_formatter.py | 8 +-- tests/formatters/test_manifest_formatter.py | 10 ++-- tests/resources/pyproject.toml | 3 +- tests/test_config.py | 20 +++++++ tests/test_scoring.py | 53 +++++++++++-------- 9 files changed, 93 insertions(+), 58 deletions(-) diff --git a/src/dbt_score/config.py b/src/dbt_score/config.py index b061281..ed42b06 100644 --- a/src/dbt_score/config.py +++ b/src/dbt_score/config.py @@ -81,7 +81,6 @@ def load(self) -> None: self._load_toml_file(str(config_file)) self.validate() - def validate(self) -> None: """Validate the config.""" if self.bronze_medal_threshold >= self.silver_medal_threshold: @@ -93,8 +92,6 @@ def validate(self) -> None: "silver_medal_threshold must be lower than gold_medal_threshold" ) - - def overload(self, values: dict[str, Any]) -> None: """Overload config with additional values.""" for key, value in values.items(): diff --git a/src/dbt_score/evaluation.py b/src/dbt_score/evaluation.py index f79b629..81f9a93 100644 --- a/src/dbt_score/evaluation.py +++ b/src/dbt_score/evaluation.py @@ -21,11 +21,11 @@ class Evaluation: """Evaluate a set of rules on a set of nodes.""" def __init__( - self, - rule_registry: RuleRegistry, - manifest_loader: ManifestLoader, - formatter: Formatter, - scorer: Scorer, + self, + rule_registry: RuleRegistry, + manifest_loader: ManifestLoader, + formatter: Formatter, + scorer: Scorer, ) -> None: """Create an Evaluation object. diff --git a/src/dbt_score/formatters/manifest_formatter.py b/src/dbt_score/formatters/manifest_formatter.py index e6d3c45..e474410 100644 --- a/src/dbt_score/formatters/manifest_formatter.py +++ b/src/dbt_score/formatters/manifest_formatter.py @@ -2,7 +2,7 @@ import copy import json -from typing import Any +from typing import Any, TypedDict from dbt_score.evaluation import ModelResultsType from dbt_score.formatters import Formatter @@ -14,18 +14,20 @@ class ManifestFormatter(Formatter): def __init__(self, *args: Any, **kwargs: Any) -> None: """Instantiate a manifest formatter.""" - self._model_scores: dict[str, float] = {} + result_dict = TypedDict("result_dict", {"score": float, "medal": str}) + self._model_results: dict[str, result_dict] = {} super().__init__(*args, **kwargs) def model_evaluated( - self, model: Model, results: ModelResultsType, score: float + self, model: Model, results: ModelResultsType, score: float, medal: str ) -> None: """Callback when a model has been evaluated.""" - self._model_scores[model.unique_id] = score + self._model_results[model.unique_id] = {"score": score, "medal": medal} - def project_evaluated(self, score: float) -> None: + def project_evaluated(self, score: float, medal: str) -> None: """Callback when a project has been evaluated.""" manifest = copy.copy(self._manifest_loader.raw_manifest) - for model_id, score in self._model_scores.items(): - manifest["nodes"][model_id]["meta"]["score"] = round(score, 1) + for model_id, results in self._model_results.items(): + manifest["nodes"][model_id]["meta"]["score"] = round(results["score"], 1) + manifest["nodes"][model_id]["meta"]["medal"] = results["medal"] print(json.dumps(manifest, indent=2)) diff --git a/src/dbt_score/scoring.py b/src/dbt_score/scoring.py index e176645..7f7a046 100644 --- a/src/dbt_score/scoring.py +++ b/src/dbt_score/scoring.py @@ -24,11 +24,15 @@ class Scorer: min_score = 0.0 max_score = 10.0 + # Medals + gold_medal = "🥇" + silver_medal = "🥈" + bronze_medal = "🥉" + no_medal = "🤡" + def __init__(self, config: Config) -> None: """Create a Scorer object.""" - self.bronze_medal_threshold = config.bronze_medal_threshold - self.silver_medal_threshold = config.silver_medal_threshold - self.gold_medal_threshold = config.gold_medal_threshold + self.config = config def score_model(self, model_results: ModelResultsType) -> float: """Compute the score of a given model.""" @@ -36,8 +40,8 @@ def score_model(self, model_results: ModelResultsType) -> float: # No rule? No problem return self.max_score if any( - rule.severity == Severity.CRITICAL and isinstance(result, RuleViolation) - for rule, result in model_results.items() + rule.severity == Severity.CRITICAL and isinstance(result, RuleViolation) + for rule, result in model_results.items() ): # If there's a CRITICAL violation, the score is 0 return self.min_score @@ -69,11 +73,11 @@ def score_aggregate_models(self, scores: list[float]) -> float: def award_medal(self, score: float) -> str: """Award a medal based on a score.""" rounded_score = round(score, 1) - if rounded_score >= self.gold_medal_threshold: - return "🥇" - elif rounded_score >= self.silver_medal_threshold: - return "🥈" - elif rounded_score >= self.bronze_medal_threshold: - return "🥉" + if rounded_score >= self.config.gold_medal_threshold: + return self.gold_medal + elif rounded_score >= self.config.silver_medal_threshold: + return self.silver_medal + elif rounded_score >= self.config.bronze_medal_threshold: + return self.bronze_medal else: - return "" + return self.no_medal diff --git a/tests/formatters/test_human_readable_formatter.py b/tests/formatters/test_human_readable_formatter.py index 0cd0460..d6d58c6 100644 --- a/tests/formatters/test_human_readable_formatter.py +++ b/tests/formatters/test_human_readable_formatter.py @@ -21,7 +21,7 @@ def test_human_readable_formatter_model( rule_severity_medium: Exception("Oh noes"), rule_severity_critical: RuleViolation("Error"), } - formatter.model_evaluated(model1, results, 10.0) + formatter.model_evaluated(model1, results, 10.0, "🥇") stdout = capsys.readouterr().out assert ( stdout @@ -29,7 +29,7 @@ def test_human_readable_formatter_model( \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 -Score: \x1B[1m10.0\x1B[0m +Score: \x1B[1m10.0\x1B[0m 🥇 """ ) @@ -38,6 +38,6 @@ def test_human_readable_formatter_model( def test_human_readable_formatter_project(capsys, manifest_loader): """Ensure the formatter has the correct output after project evaluation.""" formatter = HumanReadableFormatter(manifest_loader=manifest_loader) - formatter.project_evaluated(10.0) + formatter.project_evaluated(10.0, "🥇") stdout = capsys.readouterr().out - assert stdout == "Project score: \x1B[1m10.0\x1B[0m\n" + assert stdout == "Project score: \x1B[1m10.0\x1B[0m 🥇\n" diff --git a/tests/formatters/test_manifest_formatter.py b/tests/formatters/test_manifest_formatter.py index 7403e5f..63a452b 100644 --- a/tests/formatters/test_manifest_formatter.py +++ b/tests/formatters/test_manifest_formatter.py @@ -22,7 +22,7 @@ def test_manifest_formatter_model( rule_severity_medium: Exception("Oh noes"), rule_severity_critical: RuleViolation("Error"), } - formatter.model_evaluated(model1, results, 10.0) + formatter.model_evaluated(model1, results, 10.0, "🥇") stdout = capsys.readouterr().out assert stdout == "" @@ -49,10 +49,12 @@ def test_manifest_formatter_project( rule_severity_critical: None, } - formatter.model_evaluated(model1, result1, 5.0) - formatter.model_evaluated(model2, result2, 10.0) - formatter.project_evaluated(7.5) + formatter.model_evaluated(model1, result1, 5.0, "🤡") + formatter.model_evaluated(model2, result2, 10.0, "🥇") + formatter.project_evaluated(7.5, "🥉") stdout = capsys.readouterr().out new_manifest = json.loads(stdout) assert new_manifest["nodes"]["model.package.model1"]["meta"]["score"] == 5.0 + assert new_manifest["nodes"]["model.package.model1"]["meta"]["medal"] == "🤡" assert new_manifest["nodes"]["model.package.model2"]["meta"]["score"] == 10.0 + assert new_manifest["nodes"]["model.package.model2"]["meta"]["medal"] == "🥇" diff --git a/tests/resources/pyproject.toml b/tests/resources/pyproject.toml index ee8d495..2f5242f 100644 --- a/tests/resources/pyproject.toml +++ b/tests/resources/pyproject.toml @@ -1,7 +1,8 @@ [tool.dbt-score] rule_namespaces = ["foo", "tests"] disabled_rules = ["foo.foo", "tests.bar"] - +bronze_medal_threshold = 6.0 +silver_medal_threshold = 7.0 [tool.dbt-score.rules."foo.bar"] severity=4 diff --git a/tests/test_config.py b/tests/test_config.py index e85730e..fbf2cd9 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -17,6 +17,9 @@ def test_load_valid_toml_file(valid_config_path): config.rules_config["tests.rules.example.rule_test_example"].severity == Severity.CRITICAL ) + assert config.bronze_medal_threshold == 6.0 + assert config.silver_medal_threshold == 7.0 + assert config.gold_medal_threshold == 9.0 def test_load_invalid_toml_file(caplog, invalid_config_path): @@ -37,6 +40,23 @@ def test_invalid_rule_config(rule_severity_low): rule_severity_low(config) +def test_invalid_medal_thresholds(): + """Test that invalid medal thresholds raises an exception.""" + config = Config() + config.bronze_medal_threshold = 9.0 + config.silver_medal_threshold = 8.0 + config.gold_medal_threshold = 10.0 + with pytest.raises(ValueError, match="bronze_medal_threshold must be lower than"): + config.validate() + + config = Config() + config.bronze_medal_threshold = 8.0 + config.silver_medal_threshold = 9.5 + config.gold_medal_threshold = 9.5 + with pytest.raises(ValueError, match="silver_medal_threshold must be lower than"): + config.validate() + + def test_valid_rule_config(valid_config_path, rule_with_config): """Test that a valid rule config can be loaded.""" config = RuleConfig(severity=Severity(4), config={"model_name": "baz"}) diff --git a/tests/test_scoring.py b/tests/test_scoring.py index cb50eaa..48c3364 100644 --- a/tests/test_scoring.py +++ b/tests/test_scoring.py @@ -5,15 +5,15 @@ from dbt_score.scoring import Scorer -def test_scorer_model_no_results(): +def test_scorer_model_no_results(default_config): """Test scorer with a model without any result.""" - scorer = Scorer() + scorer = Scorer(config=default_config) assert scorer.score_model({}) == 10.0 -def test_scorer_model_severity_low(rule_severity_low): +def test_scorer_model_severity_low(default_config, rule_severity_low): """Test scorer with a model and one low severity rule.""" - scorer = Scorer() + scorer = Scorer(config=default_config) assert scorer.score_model({rule_severity_low: None}) == 10.0 assert scorer.score_model({rule_severity_low: Exception()}) == 10.0 assert ( @@ -22,9 +22,9 @@ def test_scorer_model_severity_low(rule_severity_low): ) -def test_scorer_model_severity_medium(rule_severity_medium): +def test_scorer_model_severity_medium(default_config, rule_severity_medium): """Test scorer with a model and one medium severity rule.""" - scorer = Scorer() + scorer = Scorer(config=default_config) assert scorer.score_model({rule_severity_medium: None}) == 10.0 assert scorer.score_model({rule_severity_medium: Exception()}) == 10.0 assert ( @@ -33,27 +33,27 @@ def test_scorer_model_severity_medium(rule_severity_medium): ) -def test_scorer_model_severity_high(rule_severity_high): +def test_scorer_model_severity_high(default_config, rule_severity_high): """Test scorer with a model and one high severity rule.""" - scorer = Scorer() + scorer = Scorer(config=default_config) assert scorer.score_model({rule_severity_high: None}) == 10.0 assert scorer.score_model({rule_severity_high: Exception()}) == 10.0 assert scorer.score_model({rule_severity_high: RuleViolation("error")}) == 0.0 -def test_scorer_model_severity_critical(rule_severity_critical): +def test_scorer_model_severity_critical(default_config, rule_severity_critical): """Test scorer with a model and one critical severity rule.""" - scorer = Scorer() + scorer = Scorer(config=default_config) assert scorer.score_model({rule_severity_critical: None}) == 10.0 assert scorer.score_model({rule_severity_critical: Exception()}) == 10.0 assert scorer.score_model({rule_severity_critical: RuleViolation("error")}) == 0.0 def test_scorer_model_severity_critical_overwrites( - rule_severity_low, rule_severity_critical + default_config, rule_severity_low, rule_severity_critical ): """Test scorer with a model and multiple rules including one critical.""" - scorer = Scorer() + scorer = Scorer(config=default_config) assert ( scorer.score_model( {rule_severity_low: None, rule_severity_critical: RuleViolation("error")} @@ -63,10 +63,10 @@ def test_scorer_model_severity_critical_overwrites( def test_scorer_model_multiple_rules( - rule_severity_low, rule_severity_medium, rule_severity_high + default_config, rule_severity_low, rule_severity_medium, rule_severity_high ): """Test scorer with a model and multiple rules.""" - scorer = Scorer() + scorer = Scorer(config=default_config) assert ( round( scorer.score_model( @@ -110,27 +110,36 @@ def test_scorer_model_multiple_rules( ) -def test_scorer_aggregate_empty(): +def test_scorer_aggregate_empty(default_config): """Test scorer aggregation with no results.""" - scorer = Scorer() + scorer = Scorer(config=default_config) assert scorer.score_aggregate_models([]) == 10.0 -def test_scorer_aggregate_with_0(): +def test_scorer_aggregate_with_0(default_config): """Test scorer aggregation with one result that is 0.0.""" - scorer = Scorer() + scorer = Scorer(config=default_config) assert scorer.score_aggregate_models([1.0, 5.0, 0.0]) == 0.0 -def test_scorer_aggregate_single(): +def test_scorer_aggregate_single(default_config): """Test scorer aggregation with a single results.""" - scorer = Scorer() + scorer = Scorer(config=default_config) assert scorer.score_aggregate_models([4.2]) == 4.2 -def test_scorer_aggregate_multiple(): +def test_scorer_aggregate_multiple(default_config): """Test scorer aggregation with multiple results.""" - scorer = Scorer() + scorer = Scorer(config=default_config) assert scorer.score_aggregate_models([1.0, 1.0, 1.0]) == 1.0 assert scorer.score_aggregate_models([0.0, 0.0, 0.0]) == 0.0 assert scorer.score_aggregate_models([1.0, 7.4, 4.2]) == 4.2 + + +def test_award_medal(default_config): + """Test scorer awarding a medal.""" + scorer = Scorer(config=default_config) + assert scorer.award_medal(9.0) == "🥇" + assert scorer.award_medal(8.0) == "🥈" + assert scorer.award_medal(7.0) == "🥉" + assert scorer.award_medal(4.0) == "🤡" From cd23b320ddfc55f13c938712fffd048fc1723563 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Tue, 28 May 2024 12:19:14 +0200 Subject: [PATCH 04/21] Refactor model configuration and scoring --- src/dbt_score/config.py | 70 ++++++++---- src/dbt_score/evaluation.py | 12 +- src/dbt_score/formatters/__init__.py | 6 +- .../formatters/human_readable_formatter.py | 9 +- .../formatters/manifest_formatter.py | 18 +-- src/dbt_score/lint.py | 2 +- src/dbt_score/scoring.py | 69 ++++++------ .../test_human_readable_formatter.py | 5 +- tests/formatters/test_manifest_formatter.py | 9 +- tests/resources/pyproject.toml | 8 +- tests/test_config.py | 34 +++--- tests/test_scoring.py | 103 +++++++++++------- 12 files changed, 205 insertions(+), 140 deletions(-) diff --git a/src/dbt_score/config.py b/src/dbt_score/config.py index ed42b06..d99a0f1 100644 --- a/src/dbt_score/config.py +++ b/src/dbt_score/config.py @@ -2,6 +2,7 @@ import logging import tomllib +from dataclasses import dataclass from pathlib import Path from typing import Any, Final @@ -12,28 +13,63 @@ DEFAULT_CONFIG_FILE = "pyproject.toml" +@dataclass +class MedalConfig: + """Configuration for medals.""" + + bronze_icon: str = "🥉" + silver_icon: str = "🥈" + gold_icon: str = "🥇" + wip_icon: str = "🚧" + bronze_threshold: float = 6.0 + silver_threshold: float = 8.0 + gold_threshold: float = 10.0 + + @classmethod + def load_from_dict(cls, medal_config: dict[str, Any]) -> "MedalConfig": + """Create a MedalConfig from a dictionary.""" + options = {} + for medal, medal_options in medal_config.items(): + if isinstance(medal_options, dict): + for option, value in medal_options.items(): + if not hasattr(cls, f"{medal}_{option}"): + raise AttributeError( + f"Unknown medal option: {option} for medal {medal}." + ) + options[f"{medal}_{option}"] = value + else: + logger.warning( + f"Option {medal} in tool.dbt-score.medals not supported." + ) + + return cls(**options) + + def validate(self) -> None: + """Validate the medal configuration.""" + if self.bronze_threshold >= self.silver_threshold: + raise ValueError("bronze_threshold must be lower than silver_threshold") + if self.silver_threshold >= self.gold_threshold: + raise ValueError("silver_threshold must be lower than gold_threshold") + + class Config: """Configuration for dbt-score.""" _main_section: Final[str] = "tool.dbt-score" - _options: Final[list[str]] = [ + _main_options: Final[list[str]] = [ "rule_namespaces", "disabled_rules", - "bronze_medal_threshold", - "silver_medal_threshold", - "gold_medal_threshold", ] _rules_section: Final[str] = f"{_main_section}.rules" + _medal_section: Final[str] = f"{_main_section}.medals" def __init__(self) -> None: """Initialize the Config object.""" self.rule_namespaces: list[str] = ["dbt_score.rules", "dbt_score_rules"] self.disabled_rules: list[str] = [] - self.bronze_medal_threshold: float = 7.0 - self.silver_medal_threshold: float = 8.0 - self.gold_medal_threshold: float = 9.0 self.rules_config: dict[str, RuleConfig] = {} self.config_file: Path | None = None + self.medal_config: MedalConfig = MedalConfig() def set_option(self, option: str, value: Any) -> None: """Set an option in the config.""" @@ -47,10 +83,11 @@ def _load_toml_file(self, file: str) -> None: tools = toml_data.get("tool", {}) dbt_score_config = tools.get("dbt-score", {}) rules_config = dbt_score_config.pop("rules", {}) + medal_config = dbt_score_config.pop("medals", {}) # Main configuration for option, value in dbt_score_config.items(): - if option in self._options: + if option in self._main_options: self.set_option(option, value) elif not isinstance( value, dict @@ -59,6 +96,11 @@ def _load_toml_file(self, file: str) -> None: f"Option {option} in {self._main_section} not supported." ) + # Medal configuration + if medal_config: + self.medal_config = self.medal_config.load_from_dict(medal_config) + self.medal_config.validate() + # Rule configuration self.rules_config = { name: RuleConfig.from_dict(config) for name, config in rules_config.items() @@ -79,18 +121,6 @@ def load(self) -> None: config_file = self.get_config_file(Path.cwd()) if config_file: self._load_toml_file(str(config_file)) - self.validate() - - def validate(self) -> None: - """Validate the config.""" - if self.bronze_medal_threshold >= self.silver_medal_threshold: - raise ValueError( - "bronze_medal_threshold must be lower than silver_medal_threshold" - ) - if self.silver_medal_threshold >= self.gold_medal_threshold: - raise ValueError( - "silver_medal_threshold must be lower than gold_medal_threshold" - ) def overload(self, values: dict[str, Any]) -> None: """Overload config with additional values.""" diff --git a/src/dbt_score/evaluation.py b/src/dbt_score/evaluation.py index 81f9a93..41b5e0f 100644 --- a/src/dbt_score/evaluation.py +++ b/src/dbt_score/evaluation.py @@ -8,7 +8,7 @@ from dbt_score.models import ManifestLoader, Model from dbt_score.rule import Rule, RuleViolation from dbt_score.rule_registry import RuleRegistry -from dbt_score.scoring import Scorer +from dbt_score.scoring import Score, Scorer # The results of a given model are stored in a dictionary, mapping rules to either: # - None if there was no issue @@ -44,10 +44,10 @@ def __init__( self.results: dict[Model, ModelResultsType] = {} # For each model, its computed score - self.scores: dict[Model, float] = {} + self.scores: dict[Model, Score] = {} # The aggregated project score - self.project_score: float + self.project_score: Score def evaluate(self) -> None: """Evaluate all rules.""" @@ -64,14 +64,12 @@ def evaluate(self) -> None: self.results[model][rule.__class__] = result self.scores[model] = self._scorer.score_model(self.results[model]) - model_medal = self._scorer.award_medal(self.scores[model]) self._formatter.model_evaluated( - model, self.results[model], self.scores[model], model_medal + model, self.results[model], self.scores[model] ) # Compute score for project self.project_score = self._scorer.score_aggregate_models( list(self.scores.values()) ) - project_medal = self._scorer.award_medal(self.project_score) - self._formatter.project_evaluated(self.project_score, project_medal) + self._formatter.project_evaluated(self.project_score) diff --git a/src/dbt_score/formatters/__init__.py b/src/dbt_score/formatters/__init__.py index 023b487..09466e1 100644 --- a/src/dbt_score/formatters/__init__.py +++ b/src/dbt_score/formatters/__init__.py @@ -5,6 +5,8 @@ import typing from abc import ABC, abstractmethod +from dbt_score.scoring import Score + if typing.TYPE_CHECKING: from dbt_score.evaluation import ModelResultsType from dbt_score.models import ManifestLoader, Model @@ -19,12 +21,12 @@ def __init__(self, manifest_loader: ManifestLoader): @abstractmethod def model_evaluated( - self, model: Model, results: ModelResultsType, score: float, medal: str + self, model: Model, results: ModelResultsType, score: Score ) -> None: """Callback when a model has been evaluated.""" raise NotImplementedError @abstractmethod - def project_evaluated(self, score: float, medal: str) -> None: + def project_evaluated(self, score: Score) -> None: """Callback when a project has been evaluated.""" raise NotImplementedError diff --git a/src/dbt_score/formatters/human_readable_formatter.py b/src/dbt_score/formatters/human_readable_formatter.py index 7689668..217422d 100644 --- a/src/dbt_score/formatters/human_readable_formatter.py +++ b/src/dbt_score/formatters/human_readable_formatter.py @@ -5,6 +5,7 @@ from dbt_score.formatters import Formatter from dbt_score.models import Model from dbt_score.rule import RuleViolation +from dbt_score.scoring import Score class HumanReadableFormatter(Formatter): @@ -21,7 +22,7 @@ def bold(text: str) -> str: return f"\033[1m{text}\033[0m" def model_evaluated( - self, model: Model, results: ModelResultsType, score: float, medal: str + self, model: Model, results: ModelResultsType, score: Score ) -> None: """Callback when a model has been evaluated.""" print(f"Model {self.bold(model.name)}") @@ -35,9 +36,9 @@ def model_evaluated( ) else: print(f"{self.indent}{self.label_error} {rule.source()}: {result!s}") - print(f"Score: {self.bold(str(round(score, 1)))} {medal}") + print(f"Score: {self.bold(str(round(score.score, 1)))} {score.medal}") print() - def project_evaluated(self, score: float, medal: str) -> None: + def project_evaluated(self, score: Score) -> None: """Callback when a project has been evaluated.""" - print(f"Project score: {self.bold(str(round(score, 1)))} {medal}") + print(f"Project score: {self.bold(str(round(score.score, 1)))} {score.medal}") diff --git a/src/dbt_score/formatters/manifest_formatter.py b/src/dbt_score/formatters/manifest_formatter.py index e474410..96d481d 100644 --- a/src/dbt_score/formatters/manifest_formatter.py +++ b/src/dbt_score/formatters/manifest_formatter.py @@ -2,11 +2,12 @@ import copy import json -from typing import Any, TypedDict +from typing import Any from dbt_score.evaluation import ModelResultsType from dbt_score.formatters import Formatter from dbt_score.models import Model +from dbt_score.scoring import Score class ManifestFormatter(Formatter): @@ -14,20 +15,19 @@ class ManifestFormatter(Formatter): def __init__(self, *args: Any, **kwargs: Any) -> None: """Instantiate a manifest formatter.""" - result_dict = TypedDict("result_dict", {"score": float, "medal": str}) - self._model_results: dict[str, result_dict] = {} + self._model_scores: dict[str, Score] = {} super().__init__(*args, **kwargs) def model_evaluated( - self, model: Model, results: ModelResultsType, score: float, medal: str + self, model: Model, results: ModelResultsType, score: Score ) -> None: """Callback when a model has been evaluated.""" - self._model_results[model.unique_id] = {"score": score, "medal": medal} + self._model_scores[model.unique_id] = score - def project_evaluated(self, score: float, medal: str) -> None: + def project_evaluated(self, score: Score) -> None: """Callback when a project has been evaluated.""" manifest = copy.copy(self._manifest_loader.raw_manifest) - for model_id, results in self._model_results.items(): - manifest["nodes"][model_id]["meta"]["score"] = round(results["score"], 1) - manifest["nodes"][model_id]["meta"]["medal"] = results["medal"] + for model_id, model_score in self._model_scores.items(): + manifest["nodes"][model_id]["meta"]["score"] = model_score.score + manifest["nodes"][model_id]["meta"]["medal"] = model_score.medal print(json.dumps(manifest, indent=2)) diff --git a/src/dbt_score/lint.py b/src/dbt_score/lint.py index 78e6c04..5b1755a 100644 --- a/src/dbt_score/lint.py +++ b/src/dbt_score/lint.py @@ -30,7 +30,7 @@ def lint_dbt_project( formatters = {"plain": HumanReadableFormatter, "manifest": ManifestFormatter} formatter = formatters[format](manifest_loader=manifest_loader) - scorer = Scorer(config) + scorer = Scorer(config.medal_config) evaluation = Evaluation( rule_registry=rule_registry, diff --git a/src/dbt_score/scoring.py b/src/dbt_score/scoring.py index 7f7a046..a02edaf 100644 --- a/src/dbt_score/scoring.py +++ b/src/dbt_score/scoring.py @@ -3,14 +3,23 @@ from __future__ import annotations import typing +from dataclasses import dataclass -from dbt_score.config import Config +from dbt_score.config import MedalConfig if typing.TYPE_CHECKING: from dbt_score.evaluation import ModelResultsType from dbt_score.rule import RuleViolation, Severity +@dataclass +class Score: + """Class representing a score.""" + + score: float + medal: str + + class Scorer: """Logic for computing scores.""" @@ -24,30 +33,24 @@ class Scorer: min_score = 0.0 max_score = 10.0 - # Medals - gold_medal = "🥇" - silver_medal = "🥈" - bronze_medal = "🥉" - no_medal = "🤡" - - def __init__(self, config: Config) -> None: + def __init__(self, medal_config: MedalConfig) -> None: """Create a Scorer object.""" - self.config = config + self._medal_config = medal_config - def score_model(self, model_results: ModelResultsType) -> float: + def score_model(self, model_results: ModelResultsType) -> Score: """Compute the score of a given model.""" if len(model_results) == 0: # No rule? No problem - return self.max_score - if any( + score = self.max_score + elif any( rule.severity == Severity.CRITICAL and isinstance(result, RuleViolation) for rule, result in model_results.items() ): # If there's a CRITICAL violation, the score is 0 - return self.min_score + score = self.min_score else: # Otherwise, the score is the weighted average (by severity) of the results - return ( + score = ( sum( [ # The more severe the violation, the more points are lost @@ -61,23 +64,27 @@ def score_model(self, model_results: ModelResultsType) -> float: * self.max_score ) - def score_aggregate_models(self, scores: list[float]) -> float: + return Score(score, self._medal(score)) + + def score_aggregate_models(self, scores: list[Score]) -> Score: """Compute the score of a list of models.""" - if 0.0 in scores: + actual_scores = [s.score for s in scores] + if 0.0 in actual_scores: # Any model with a CRITICAL violation makes the project score 0 - return self.min_score - if len(scores) == 0: - return self.max_score - return sum(scores) / len(scores) - - def award_medal(self, score: float) -> str: - """Award a medal based on a score.""" - rounded_score = round(score, 1) - if rounded_score >= self.config.gold_medal_threshold: - return self.gold_medal - elif rounded_score >= self.config.silver_medal_threshold: - return self.silver_medal - elif rounded_score >= self.config.bronze_medal_threshold: - return self.bronze_medal + score = Score(self.min_score, self._medal(self.min_score)) + elif len(actual_scores) == 0: + score = Score(self.max_score, self._medal(self.max_score)) else: - return self.no_medal + average_score = sum(actual_scores) / len(actual_scores) + score = Score(average_score, self._medal(average_score)) + return score + + def _medal(self, score: float) -> str: + """Compute the medal of a given score.""" + if score >= self._medal_config.gold_threshold: + return self._medal_config.gold_icon + elif score >= self._medal_config.silver_threshold: + return self._medal_config.silver_icon + elif score >= self._medal_config.bronze_threshold: + return self._medal_config.bronze_icon + return self._medal_config.wip_icon diff --git a/tests/formatters/test_human_readable_formatter.py b/tests/formatters/test_human_readable_formatter.py index d6d58c6..c058f54 100644 --- a/tests/formatters/test_human_readable_formatter.py +++ b/tests/formatters/test_human_readable_formatter.py @@ -4,6 +4,7 @@ from dbt_score.formatters.human_readable_formatter import HumanReadableFormatter from dbt_score.rule import Rule, RuleViolation +from dbt_score.scoring import Score def test_human_readable_formatter_model( @@ -21,7 +22,7 @@ def test_human_readable_formatter_model( rule_severity_medium: Exception("Oh noes"), rule_severity_critical: RuleViolation("Error"), } - formatter.model_evaluated(model1, results, 10.0, "🥇") + formatter.model_evaluated(model1, results, Score(10.0, "🥇")) stdout = capsys.readouterr().out assert ( stdout @@ -38,6 +39,6 @@ def test_human_readable_formatter_model( def test_human_readable_formatter_project(capsys, manifest_loader): """Ensure the formatter has the correct output after project evaluation.""" formatter = HumanReadableFormatter(manifest_loader=manifest_loader) - formatter.project_evaluated(10.0, "🥇") + formatter.project_evaluated(Score(10.0, "🥇")) stdout = capsys.readouterr().out assert stdout == "Project score: \x1B[1m10.0\x1B[0m 🥇\n" diff --git a/tests/formatters/test_manifest_formatter.py b/tests/formatters/test_manifest_formatter.py index 63a452b..1f9ddf1 100644 --- a/tests/formatters/test_manifest_formatter.py +++ b/tests/formatters/test_manifest_formatter.py @@ -5,6 +5,7 @@ from dbt_score.formatters.manifest_formatter import ManifestFormatter from dbt_score.rule import Rule, RuleViolation +from dbt_score.scoring import Score def test_manifest_formatter_model( @@ -22,7 +23,7 @@ def test_manifest_formatter_model( rule_severity_medium: Exception("Oh noes"), rule_severity_critical: RuleViolation("Error"), } - formatter.model_evaluated(model1, results, 10.0, "🥇") + formatter.model_evaluated(model1, results, Score(10.0, "🥇")) stdout = capsys.readouterr().out assert stdout == "" @@ -49,9 +50,9 @@ def test_manifest_formatter_project( rule_severity_critical: None, } - formatter.model_evaluated(model1, result1, 5.0, "🤡") - formatter.model_evaluated(model2, result2, 10.0, "🥇") - formatter.project_evaluated(7.5, "🥉") + formatter.model_evaluated(model1, result1, Score(5.0, "🤡")) + formatter.model_evaluated(model2, result2, Score(10.0, "🥇")) + formatter.project_evaluated(Score(7.5, "🥉")) stdout = capsys.readouterr().out new_manifest = json.loads(stdout) assert new_manifest["nodes"]["model.package.model1"]["meta"]["score"] == 5.0 diff --git a/tests/resources/pyproject.toml b/tests/resources/pyproject.toml index 2f5242f..dc4d8f2 100644 --- a/tests/resources/pyproject.toml +++ b/tests/resources/pyproject.toml @@ -1,8 +1,12 @@ [tool.dbt-score] rule_namespaces = ["foo", "tests"] disabled_rules = ["foo.foo", "tests.bar"] -bronze_medal_threshold = 6.0 -silver_medal_threshold = 7.0 + +[tool.dbt-score.medals] +bronze.threshold = 6.0 + +[tool.dbt-score.medals.silver] +threshold = 7.0 [tool.dbt-score.rules."foo.bar"] severity=4 diff --git a/tests/test_config.py b/tests/test_config.py index fbf2cd9..b85e460 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -2,7 +2,7 @@ from pathlib import Path import pytest -from dbt_score.config import Config +from dbt_score.config import Config, MedalConfig from dbt_score.rule import RuleConfig, Severity @@ -17,9 +17,9 @@ def test_load_valid_toml_file(valid_config_path): config.rules_config["tests.rules.example.rule_test_example"].severity == Severity.CRITICAL ) - assert config.bronze_medal_threshold == 6.0 - assert config.silver_medal_threshold == 7.0 - assert config.gold_medal_threshold == 9.0 + assert config.medal_config.bronze_threshold == 6.0 + assert config.medal_config.silver_threshold == 7.0 + assert config.medal_config.gold_threshold == 10.0 def test_load_invalid_toml_file(caplog, invalid_config_path): @@ -42,19 +42,19 @@ def test_invalid_rule_config(rule_severity_low): def test_invalid_medal_thresholds(): """Test that invalid medal thresholds raises an exception.""" - config = Config() - config.bronze_medal_threshold = 9.0 - config.silver_medal_threshold = 8.0 - config.gold_medal_threshold = 10.0 - with pytest.raises(ValueError, match="bronze_medal_threshold must be lower than"): - config.validate() - - config = Config() - config.bronze_medal_threshold = 8.0 - config.silver_medal_threshold = 9.5 - config.gold_medal_threshold = 9.5 - with pytest.raises(ValueError, match="silver_medal_threshold must be lower than"): - config.validate() + medal_config = MedalConfig() + medal_config.bronze_threshold = 9.0 + medal_config.silver_threshold = 8.0 + medal_config.gold_threshold = 10.0 + with pytest.raises(ValueError, match="bronze_threshold must be lower than"): + medal_config.validate() + + medal_config = MedalConfig() + medal_config.bronze_threshold = 8.0 + medal_config.silver_threshold = 9.5 + medal_config.gold_threshold = 9.5 + with pytest.raises(ValueError, match="silver_threshold must be lower than"): + medal_config.validate() def test_valid_rule_config(valid_config_path, rule_with_config): diff --git a/tests/test_scoring.py b/tests/test_scoring.py index 48c3364..d7cbdeb 100644 --- a/tests/test_scoring.py +++ b/tests/test_scoring.py @@ -2,62 +2,67 @@ from dbt_score.rule import RuleViolation -from dbt_score.scoring import Scorer +from dbt_score.scoring import Score, Scorer def test_scorer_model_no_results(default_config): """Test scorer with a model without any result.""" - scorer = Scorer(config=default_config) - assert scorer.score_model({}) == 10.0 + scorer = Scorer(medal_config=default_config.medal_config) + assert scorer.score_model({}).score == 10.0 def test_scorer_model_severity_low(default_config, rule_severity_low): """Test scorer with a model and one low severity rule.""" - scorer = Scorer(config=default_config) - assert scorer.score_model({rule_severity_low: None}) == 10.0 - assert scorer.score_model({rule_severity_low: Exception()}) == 10.0 + scorer = Scorer(medal_config=default_config.medal_config) + assert scorer.score_model({rule_severity_low: None}).score == 10.0 + assert scorer.score_model({rule_severity_low: Exception()}).score == 10.0 assert ( - round(scorer.score_model({rule_severity_low: RuleViolation("error")}), 2) + round(scorer.score_model({rule_severity_low: RuleViolation("error")}).score, 2) == 6.67 ) def test_scorer_model_severity_medium(default_config, rule_severity_medium): """Test scorer with a model and one medium severity rule.""" - scorer = Scorer(config=default_config) - assert scorer.score_model({rule_severity_medium: None}) == 10.0 - assert scorer.score_model({rule_severity_medium: Exception()}) == 10.0 + scorer = Scorer(medal_config=default_config.medal_config) + assert scorer.score_model({rule_severity_medium: None}).score == 10.0 + assert scorer.score_model({rule_severity_medium: Exception()}).score == 10.0 assert ( - round(scorer.score_model({rule_severity_medium: RuleViolation("error")}), 2) + round( + scorer.score_model({rule_severity_medium: RuleViolation("error")}).score, 2 + ) == 3.33 ) def test_scorer_model_severity_high(default_config, rule_severity_high): """Test scorer with a model and one high severity rule.""" - scorer = Scorer(config=default_config) - assert scorer.score_model({rule_severity_high: None}) == 10.0 - assert scorer.score_model({rule_severity_high: Exception()}) == 10.0 - assert scorer.score_model({rule_severity_high: RuleViolation("error")}) == 0.0 + scorer = Scorer(medal_config=default_config.medal_config) + assert scorer.score_model({rule_severity_high: None}).score == 10.0 + assert scorer.score_model({rule_severity_high: Exception()}).score == 10.0 + assert scorer.score_model({rule_severity_high: RuleViolation("error")}).score == 0.0 def test_scorer_model_severity_critical(default_config, rule_severity_critical): """Test scorer with a model and one critical severity rule.""" - scorer = Scorer(config=default_config) - assert scorer.score_model({rule_severity_critical: None}) == 10.0 - assert scorer.score_model({rule_severity_critical: Exception()}) == 10.0 - assert scorer.score_model({rule_severity_critical: RuleViolation("error")}) == 0.0 + scorer = Scorer(medal_config=default_config.medal_config) + assert scorer.score_model({rule_severity_critical: None}).score == 10.0 + assert scorer.score_model({rule_severity_critical: Exception()}).score == 10.0 + assert ( + scorer.score_model({rule_severity_critical: RuleViolation("error")}).score + == 0.0 + ) def test_scorer_model_severity_critical_overwrites( default_config, rule_severity_low, rule_severity_critical ): """Test scorer with a model and multiple rules including one critical.""" - scorer = Scorer(config=default_config) + scorer = Scorer(medal_config=default_config.medal_config) assert ( scorer.score_model( {rule_severity_low: None, rule_severity_critical: RuleViolation("error")} - ) + ).score == 0.0 ) @@ -66,7 +71,7 @@ def test_scorer_model_multiple_rules( default_config, rule_severity_low, rule_severity_medium, rule_severity_high ): """Test scorer with a model and multiple rules.""" - scorer = Scorer(config=default_config) + scorer = Scorer(medal_config=default_config.medal_config) assert ( round( scorer.score_model( @@ -75,7 +80,7 @@ def test_scorer_model_multiple_rules( rule_severity_medium: Exception(), rule_severity_high: RuleViolation("error"), } - ), + ).score, 2, ) == 6.67 @@ -89,7 +94,7 @@ def test_scorer_model_multiple_rules( rule_severity_medium: RuleViolation("error"), rule_severity_high: None, } - ), + ).score, 2, ) == 7.78 @@ -103,7 +108,7 @@ def test_scorer_model_multiple_rules( rule_severity_medium: Exception(), rule_severity_high: None, } - ), + ).score, 2, ) == 8.89 @@ -112,34 +117,50 @@ def test_scorer_model_multiple_rules( def test_scorer_aggregate_empty(default_config): """Test scorer aggregation with no results.""" - scorer = Scorer(config=default_config) - assert scorer.score_aggregate_models([]) == 10.0 + scorer = Scorer(medal_config=default_config.medal_config) + assert scorer.score_aggregate_models([]).score == 10.0 def test_scorer_aggregate_with_0(default_config): """Test scorer aggregation with one result that is 0.0.""" - scorer = Scorer(config=default_config) - assert scorer.score_aggregate_models([1.0, 5.0, 0.0]) == 0.0 + scorer = Scorer(medal_config=default_config.medal_config) + scores = [Score(1.0, ""), Score(5.0, ""), Score(0.0, "")] + assert scorer.score_aggregate_models(scores).score == 0.0 def test_scorer_aggregate_single(default_config): """Test scorer aggregation with a single results.""" - scorer = Scorer(config=default_config) - assert scorer.score_aggregate_models([4.2]) == 4.2 + scorer = Scorer(medal_config=default_config.medal_config) + assert scorer.score_aggregate_models([Score(4.2, "")]).score == 4.2 def test_scorer_aggregate_multiple(default_config): """Test scorer aggregation with multiple results.""" - scorer = Scorer(config=default_config) - assert scorer.score_aggregate_models([1.0, 1.0, 1.0]) == 1.0 - assert scorer.score_aggregate_models([0.0, 0.0, 0.0]) == 0.0 - assert scorer.score_aggregate_models([1.0, 7.4, 4.2]) == 4.2 + scorer = Scorer(medal_config=default_config.medal_config) + assert ( + scorer.score_aggregate_models( + [Score(1.0, ""), Score(1.0, ""), Score(1.0, "")] + ).score + == 1.0 + ) + assert ( + scorer.score_aggregate_models( + [Score(1.0, ""), Score(7.4, ""), Score(4.2, "")] + ).score + == 4.2 + ) + assert ( + scorer.score_aggregate_models( + [Score(0.0, ""), Score(0.0, ""), Score(0.0, "")] + ).score + == 0.0 + ) -def test_award_medal(default_config): +def test_scorer_medal(default_config): """Test scorer awarding a medal.""" - scorer = Scorer(config=default_config) - assert scorer.award_medal(9.0) == "🥇" - assert scorer.award_medal(8.0) == "🥈" - assert scorer.award_medal(7.0) == "🥉" - assert scorer.award_medal(4.0) == "🤡" + scorer = Scorer(medal_config=default_config.medal_config) + assert scorer._medal(10.0) == scorer._medal_config.gold_icon + assert scorer._medal(8.0) == scorer._medal_config.silver_icon + assert scorer._medal(7.0) == scorer._medal_config.bronze_icon + assert scorer._medal(4.0) == scorer._medal_config.wip_icon From 0801af3b8b718e3b9c2ef7efb525d34437f7a9cc Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Tue, 28 May 2024 12:25:21 +0200 Subject: [PATCH 05/21] Change variable names in config --- src/dbt_score/config.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dbt_score/config.py b/src/dbt_score/config.py index d99a0f1..db536fe 100644 --- a/src/dbt_score/config.py +++ b/src/dbt_score/config.py @@ -56,12 +56,12 @@ class Config: """Configuration for dbt-score.""" _main_section: Final[str] = "tool.dbt-score" - _main_options: Final[list[str]] = [ + _options: Final[list[str]] = [ "rule_namespaces", "disabled_rules", ] - _rules_section: Final[str] = f"{_main_section}.rules" - _medal_section: Final[str] = f"{_main_section}.medals" + _rules_section: Final[str] = "rules" + _medals_section: Final[str] = "medals" def __init__(self) -> None: """Initialize the Config object.""" @@ -82,12 +82,12 @@ def _load_toml_file(self, file: str) -> None: tools = toml_data.get("tool", {}) dbt_score_config = tools.get("dbt-score", {}) - rules_config = dbt_score_config.pop("rules", {}) - medal_config = dbt_score_config.pop("medals", {}) + rules_config = dbt_score_config.pop(self._rules_section, {}) + medal_config = dbt_score_config.pop(self._medals_section, {}) # Main configuration for option, value in dbt_score_config.items(): - if option in self._main_options: + if option in self._options: self.set_option(option, value) elif not isinstance( value, dict From dbe0cfb492ff67ac321e3aab3efdb5bea9c644f4 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Wed, 29 May 2024 08:31:50 +0200 Subject: [PATCH 06/21] Improve model output --- src/dbt_score/formatters/human_readable_formatter.py | 5 +++-- tests/formatters/test_human_readable_formatter.py | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dbt_score/formatters/human_readable_formatter.py b/src/dbt_score/formatters/human_readable_formatter.py index 217422d..77340cb 100644 --- a/src/dbt_score/formatters/human_readable_formatter.py +++ b/src/dbt_score/formatters/human_readable_formatter.py @@ -25,7 +25,9 @@ def model_evaluated( self, model: Model, results: ModelResultsType, score: Score ) -> None: """Callback when a model has been evaluated.""" - print(f"Model {self.bold(model.name)}") + print( + f"{score.medal} {self.bold(model.name)} (score: {round(score.score, 1)!s})" + ) for rule, result in results.items(): if result is None: print(f"{self.indent}{self.label_ok} {rule.source()}") @@ -36,7 +38,6 @@ def model_evaluated( ) else: print(f"{self.indent}{self.label_error} {rule.source()}: {result!s}") - print(f"Score: {self.bold(str(round(score.score, 1)))} {score.medal}") print() def project_evaluated(self, score: Score) -> None: diff --git a/tests/formatters/test_human_readable_formatter.py b/tests/formatters/test_human_readable_formatter.py index c058f54..f86ee7c 100644 --- a/tests/formatters/test_human_readable_formatter.py +++ b/tests/formatters/test_human_readable_formatter.py @@ -26,11 +26,10 @@ def test_human_readable_formatter_model( stdout = capsys.readouterr().out assert ( stdout - == """Model \x1B[1mmodel1\x1B[0m + == """🥇 \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 -Score: \x1B[1m10.0\x1B[0m 🥇 """ ) From 66fb5afd0d18cac25e5843019b272a921e4e3764 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Wed, 29 May 2024 11:05:46 +0200 Subject: [PATCH 07/21] Improve model config --- src/dbt_score/config.py | 45 ++++++++++++++++++++++------------------ src/dbt_score/scoring.py | 17 ++++++++------- tests/test_config.py | 23 ++++++++++---------- tests/test_scoring.py | 8 +++---- 4 files changed, 51 insertions(+), 42 deletions(-) diff --git a/src/dbt_score/config.py b/src/dbt_score/config.py index db536fe..d1f98ff 100644 --- a/src/dbt_score/config.py +++ b/src/dbt_score/config.py @@ -2,7 +2,7 @@ import logging import tomllib -from dataclasses import dataclass +from dataclasses import asdict, dataclass, field from pathlib import Path from typing import Any, Final @@ -13,43 +13,48 @@ DEFAULT_CONFIG_FILE = "pyproject.toml" +@dataclass +class Medal: + """Medal object.""" + + icon: str + threshold: float + + @dataclass class MedalConfig: """Configuration for medals.""" - bronze_icon: str = "🥉" - silver_icon: str = "🥈" - gold_icon: str = "🥇" - wip_icon: str = "🚧" - bronze_threshold: float = 6.0 - silver_threshold: float = 8.0 - gold_threshold: float = 10.0 + bronze: Medal = field(default_factory=lambda: Medal("🥉", 6.0)) + silver: Medal = field(default_factory=lambda: Medal("🥈", 8.0)) + gold: Medal = field(default_factory=lambda: Medal("🥇", 10.0)) + wip: Medal = field(default_factory=lambda: Medal("🚧", 0.0)) @classmethod def load_from_dict(cls, medal_config: dict[str, Any]) -> "MedalConfig": """Create a MedalConfig from a dictionary.""" options = {} + default_medal_config = cls() for medal, medal_options in medal_config.items(): + if medal not in default_medal_config.__dataclass_fields__: + raise AttributeError(f"Unknown medal: {medal}.") if isinstance(medal_options, dict): - for option, value in medal_options.items(): - if not hasattr(cls, f"{medal}_{option}"): - raise AttributeError( - f"Unknown medal option: {option} for medal {medal}." - ) - options[f"{medal}_{option}"] = value + medal_defaults = asdict(default_medal_config.__getattribute__(medal)) + medal_defaults.update(medal_options) + options[medal] = Medal(**medal_defaults) else: - logger.warning( - f"Option {medal} in tool.dbt-score.medals not supported." + raise AttributeError( + f"Invalid config for medal: {medal}, must be a dictionary." ) return cls(**options) def validate(self) -> None: """Validate the medal configuration.""" - if self.bronze_threshold >= self.silver_threshold: - raise ValueError("bronze_threshold must be lower than silver_threshold") - if self.silver_threshold >= self.gold_threshold: - raise ValueError("silver_threshold must be lower than gold_threshold") + if self.bronze.threshold >= self.silver.threshold: + raise ValueError("bronze threshold must be lower than silver threshold") + if self.silver.threshold >= self.gold.threshold: + raise ValueError("silver threshold must be lower than gold threshold") class Config: diff --git a/src/dbt_score/scoring.py b/src/dbt_score/scoring.py index a02edaf..77ae681 100644 --- a/src/dbt_score/scoring.py +++ b/src/dbt_score/scoring.py @@ -81,10 +81,13 @@ def score_aggregate_models(self, scores: list[Score]) -> Score: def _medal(self, score: float) -> str: """Compute the medal of a given score.""" - if score >= self._medal_config.gold_threshold: - return self._medal_config.gold_icon - elif score >= self._medal_config.silver_threshold: - return self._medal_config.silver_icon - elif score >= self._medal_config.bronze_threshold: - return self._medal_config.bronze_icon - return self._medal_config.wip_icon + if score >= self._medal_config.gold.threshold: + return self._medal_config.gold.icon + elif score >= self._medal_config.silver.threshold: + return self._medal_config.silver.icon + elif score >= self._medal_config.bronze.threshold: + return self._medal_config.bronze.icon + elif score >= self._medal_config.wip.threshold: + return self._medal_config.wip.icon + else: + return "" diff --git a/tests/test_config.py b/tests/test_config.py index b85e460..3d377d8 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -17,9 +17,10 @@ def test_load_valid_toml_file(valid_config_path): config.rules_config["tests.rules.example.rule_test_example"].severity == Severity.CRITICAL ) - assert config.medal_config.bronze_threshold == 6.0 - assert config.medal_config.silver_threshold == 7.0 - assert config.medal_config.gold_threshold == 10.0 + assert config.medal_config.bronze.threshold == 6.0 + assert config.medal_config.silver.threshold == 7.0 + assert config.medal_config.gold.threshold == 10.0 + assert config.medal_config.wip.threshold == 0.0 def test_load_invalid_toml_file(caplog, invalid_config_path): @@ -43,17 +44,17 @@ def test_invalid_rule_config(rule_severity_low): def test_invalid_medal_thresholds(): """Test that invalid medal thresholds raises an exception.""" medal_config = MedalConfig() - medal_config.bronze_threshold = 9.0 - medal_config.silver_threshold = 8.0 - medal_config.gold_threshold = 10.0 - with pytest.raises(ValueError, match="bronze_threshold must be lower than"): + medal_config.bronze.threshold = 9.0 + medal_config.silver.threshold = 8.0 + medal_config.gold.threshold = 10.0 + with pytest.raises(ValueError, match="bronze threshold must be lower than"): medal_config.validate() medal_config = MedalConfig() - medal_config.bronze_threshold = 8.0 - medal_config.silver_threshold = 9.5 - medal_config.gold_threshold = 9.5 - with pytest.raises(ValueError, match="silver_threshold must be lower than"): + medal_config.bronze.threshold = 8.0 + medal_config.silver.threshold = 9.5 + medal_config.gold.threshold = 9.5 + with pytest.raises(ValueError, match="silver threshold must be lower than"): medal_config.validate() diff --git a/tests/test_scoring.py b/tests/test_scoring.py index d7cbdeb..46c89c8 100644 --- a/tests/test_scoring.py +++ b/tests/test_scoring.py @@ -160,7 +160,7 @@ def test_scorer_aggregate_multiple(default_config): def test_scorer_medal(default_config): """Test scorer awarding a medal.""" scorer = Scorer(medal_config=default_config.medal_config) - assert scorer._medal(10.0) == scorer._medal_config.gold_icon - assert scorer._medal(8.0) == scorer._medal_config.silver_icon - assert scorer._medal(7.0) == scorer._medal_config.bronze_icon - assert scorer._medal(4.0) == scorer._medal_config.wip_icon + assert scorer._medal(10.0) == scorer._medal_config.gold.icon + assert scorer._medal(8.0) == scorer._medal_config.silver.icon + assert scorer._medal(7.0) == scorer._medal_config.bronze.icon + assert scorer._medal(1.0) == scorer._medal_config.wip.icon From 7d48252f6e6df1c816153238a4728e500bcfb351 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Wed, 29 May 2024 14:12:19 +0200 Subject: [PATCH 08/21] Small improvements --- src/dbt_score/config.py | 12 ++++++++++-- src/dbt_score/scoring.py | 3 +++ tests/formatters/test_manifest_formatter.py | 4 ++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/dbt_score/config.py b/src/dbt_score/config.py index d1f98ff..212eea3 100644 --- a/src/dbt_score/config.py +++ b/src/dbt_score/config.py @@ -47,14 +47,23 @@ def load_from_dict(cls, medal_config: dict[str, Any]) -> "MedalConfig": f"Invalid config for medal: {medal}, must be a dictionary." ) - return cls(**options) + config = cls(**options) + config.validate() + + return config def validate(self) -> None: """Validate the medal configuration.""" + if self.wip.threshold >= self.bronze.threshold: + raise ValueError("wip threshold must be lower than bronze threshold") if self.bronze.threshold >= self.silver.threshold: raise ValueError("bronze threshold must be lower than silver threshold") if self.silver.threshold >= self.gold.threshold: raise ValueError("silver threshold must be lower than gold threshold") + if self.gold.threshold > 10.0: # noqa: PLR2004 [magic-value-comparison] + raise ValueError("gold threshold must 10.0 or lower") + if self.wip.threshold < 0.0: + raise ValueError("wip threshold must be higher than 0.0") class Config: @@ -104,7 +113,6 @@ def _load_toml_file(self, file: str) -> None: # Medal configuration if medal_config: self.medal_config = self.medal_config.load_from_dict(medal_config) - self.medal_config.validate() # Rule configuration self.rules_config = { diff --git a/src/dbt_score/scoring.py b/src/dbt_score/scoring.py index 77ae681..10b303e 100644 --- a/src/dbt_score/scoring.py +++ b/src/dbt_score/scoring.py @@ -11,6 +11,9 @@ from dbt_score.evaluation import ModelResultsType from dbt_score.rule import RuleViolation, Severity +MAX_SCORE = 10.0 +MIN_SCORE = 0.0 + @dataclass class Score: diff --git a/tests/formatters/test_manifest_formatter.py b/tests/formatters/test_manifest_formatter.py index 1f9ddf1..6deb8a6 100644 --- a/tests/formatters/test_manifest_formatter.py +++ b/tests/formatters/test_manifest_formatter.py @@ -50,12 +50,12 @@ def test_manifest_formatter_project( rule_severity_critical: None, } - formatter.model_evaluated(model1, result1, Score(5.0, "🤡")) + formatter.model_evaluated(model1, result1, Score(5.0, "🚧")) formatter.model_evaluated(model2, result2, Score(10.0, "🥇")) formatter.project_evaluated(Score(7.5, "🥉")) stdout = capsys.readouterr().out new_manifest = json.loads(stdout) assert new_manifest["nodes"]["model.package.model1"]["meta"]["score"] == 5.0 - assert new_manifest["nodes"]["model.package.model1"]["meta"]["medal"] == "🤡" + assert new_manifest["nodes"]["model.package.model1"]["meta"]["medal"] == "🚧" assert new_manifest["nodes"]["model.package.model2"]["meta"]["score"] == 10.0 assert new_manifest["nodes"]["model.package.model2"]["meta"]["medal"] == "🥇" From 218dea243119397380e718d145e87014dc43cbc9 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Thu, 30 May 2024 09:18:13 +0200 Subject: [PATCH 09/21] Update medals to badges --- src/dbt_score/config.py | 76 ++++++++++--------- .../formatters/human_readable_formatter.py | 4 +- .../formatters/manifest_formatter.py | 2 +- src/dbt_score/lint.py | 2 +- src/dbt_score/scoring.py | 39 +++++----- tests/formatters/test_manifest_formatter.py | 4 +- tests/resources/pyproject.toml | 8 +- tests/test_config.py | 40 +++++----- tests/test_scoring.py | 36 ++++----- 9 files changed, 105 insertions(+), 106 deletions(-) diff --git a/src/dbt_score/config.py b/src/dbt_score/config.py index 212eea3..b51fb34 100644 --- a/src/dbt_score/config.py +++ b/src/dbt_score/config.py @@ -14,37 +14,41 @@ @dataclass -class Medal: - """Medal object.""" +class Badge: + """Badge object.""" icon: str threshold: float @dataclass -class MedalConfig: - """Configuration for medals.""" +class BadgeConfig: + """Configuration for badges.""" - bronze: Medal = field(default_factory=lambda: Medal("🥉", 6.0)) - silver: Medal = field(default_factory=lambda: Medal("🥈", 8.0)) - gold: Medal = field(default_factory=lambda: Medal("🥇", 10.0)) - wip: Medal = field(default_factory=lambda: Medal("🚧", 0.0)) + third: Badge = field(default_factory=lambda: Badge("🥉", 6.0)) + second: Badge = field(default_factory=lambda: Badge("🥈", 8.0)) + first: Badge = field(default_factory=lambda: Badge("🥇", 10.0)) + wip_icon: str = "🚧" @classmethod - def load_from_dict(cls, medal_config: dict[str, Any]) -> "MedalConfig": - """Create a MedalConfig from a dictionary.""" - options = {} - default_medal_config = cls() - for medal, medal_options in medal_config.items(): - if medal not in default_medal_config.__dataclass_fields__: - raise AttributeError(f"Unknown medal: {medal}.") - if isinstance(medal_options, dict): - medal_defaults = asdict(default_medal_config.__getattribute__(medal)) - medal_defaults.update(medal_options) - options[medal] = Medal(**medal_defaults) + def load_from_dict(cls, badge_config: dict[str, Any]) -> "BadgeConfig": + """Create a BadgeConfig from a dictionary.""" + options: dict[str, Any] = {} + default_badge_config = cls() + for badge, badge_options in badge_config.items(): + if badge not in default_badge_config.__dataclass_fields__: + raise AttributeError(f"Unknown badge: {badge}.") + if isinstance(badge_options, dict): + badge_defaults = asdict(default_badge_config.__getattribute__(badge)) + badge_defaults.update(badge_options) + options[badge] = Badge(**badge_defaults) + # The wip icon is not a dictionary + # and is not a Badge object + elif badge == "wip_icon": + options[badge] = badge_options else: raise AttributeError( - f"Invalid config for medal: {medal}, must be a dictionary." + f"Invalid config for badge: {badge}, must be a dictionary." ) config = cls(**options) @@ -53,17 +57,15 @@ def load_from_dict(cls, medal_config: dict[str, Any]) -> "MedalConfig": return config def validate(self) -> None: - """Validate the medal configuration.""" - if self.wip.threshold >= self.bronze.threshold: - raise ValueError("wip threshold must be lower than bronze threshold") - if self.bronze.threshold >= self.silver.threshold: - raise ValueError("bronze threshold must be lower than silver threshold") - if self.silver.threshold >= self.gold.threshold: - raise ValueError("silver threshold must be lower than gold threshold") - if self.gold.threshold > 10.0: # noqa: PLR2004 [magic-value-comparison] - raise ValueError("gold threshold must 10.0 or lower") - if self.wip.threshold < 0.0: - raise ValueError("wip threshold must be higher than 0.0") + """Validate the badge configuration.""" + if self.third.threshold >= self.second.threshold: + raise ValueError("third threshold must be lower than second threshold") + if self.second.threshold >= self.first.threshold: + raise ValueError("second threshold must be lower than first threshold") + if self.first.threshold > 10.0: # noqa: PLR2004 [magic-value-comparison] + raise ValueError("first threshold must 10.0 or lower") + if self.third.threshold < 0.0: + raise ValueError("third threshold must be 0.0 or higher") class Config: @@ -75,7 +77,7 @@ class Config: "disabled_rules", ] _rules_section: Final[str] = "rules" - _medals_section: Final[str] = "medals" + _badges_section: Final[str] = "badges" def __init__(self) -> None: """Initialize the Config object.""" @@ -83,7 +85,7 @@ def __init__(self) -> None: self.disabled_rules: list[str] = [] self.rules_config: dict[str, RuleConfig] = {} self.config_file: Path | None = None - self.medal_config: MedalConfig = MedalConfig() + self.badge_config: BadgeConfig = BadgeConfig() def set_option(self, option: str, value: Any) -> None: """Set an option in the config.""" @@ -97,7 +99,7 @@ def _load_toml_file(self, file: str) -> None: tools = toml_data.get("tool", {}) dbt_score_config = tools.get("dbt-score", {}) rules_config = dbt_score_config.pop(self._rules_section, {}) - medal_config = dbt_score_config.pop(self._medals_section, {}) + badge_config = dbt_score_config.pop(self._badges_section, {}) # Main configuration for option, value in dbt_score_config.items(): @@ -110,9 +112,9 @@ def _load_toml_file(self, file: str) -> None: f"Option {option} in {self._main_section} not supported." ) - # Medal configuration - if medal_config: - self.medal_config = self.medal_config.load_from_dict(medal_config) + # Badge configuration + if badge_config: + self.badge_config = self.badge_config.load_from_dict(badge_config) # Rule configuration self.rules_config = { diff --git a/src/dbt_score/formatters/human_readable_formatter.py b/src/dbt_score/formatters/human_readable_formatter.py index 77340cb..d6cc5c5 100644 --- a/src/dbt_score/formatters/human_readable_formatter.py +++ b/src/dbt_score/formatters/human_readable_formatter.py @@ -26,7 +26,7 @@ def model_evaluated( ) -> None: """Callback when a model has been evaluated.""" print( - f"{score.medal} {self.bold(model.name)} (score: {round(score.score, 1)!s})" + f"{score.badge} {self.bold(model.name)} (score: {round(score.score, 1)!s})" ) for rule, result in results.items(): if result is None: @@ -42,4 +42,4 @@ def model_evaluated( def project_evaluated(self, score: Score) -> None: """Callback when a project has been evaluated.""" - print(f"Project score: {self.bold(str(round(score.score, 1)))} {score.medal}") + print(f"Project score: {self.bold(str(round(score.score, 1)))} {score.badge}") diff --git a/src/dbt_score/formatters/manifest_formatter.py b/src/dbt_score/formatters/manifest_formatter.py index 96d481d..01c3862 100644 --- a/src/dbt_score/formatters/manifest_formatter.py +++ b/src/dbt_score/formatters/manifest_formatter.py @@ -29,5 +29,5 @@ def project_evaluated(self, score: Score) -> None: manifest = copy.copy(self._manifest_loader.raw_manifest) for model_id, model_score in self._model_scores.items(): manifest["nodes"][model_id]["meta"]["score"] = model_score.score - manifest["nodes"][model_id]["meta"]["medal"] = model_score.medal + manifest["nodes"][model_id]["meta"]["badge"] = model_score.badge print(json.dumps(manifest, indent=2)) diff --git a/src/dbt_score/lint.py b/src/dbt_score/lint.py index 5b1755a..78e6c04 100644 --- a/src/dbt_score/lint.py +++ b/src/dbt_score/lint.py @@ -30,7 +30,7 @@ def lint_dbt_project( formatters = {"plain": HumanReadableFormatter, "manifest": ManifestFormatter} formatter = formatters[format](manifest_loader=manifest_loader) - scorer = Scorer(config.medal_config) + scorer = Scorer(config) evaluation = Evaluation( rule_registry=rule_registry, diff --git a/src/dbt_score/scoring.py b/src/dbt_score/scoring.py index 10b303e..eda9e30 100644 --- a/src/dbt_score/scoring.py +++ b/src/dbt_score/scoring.py @@ -5,22 +5,19 @@ import typing from dataclasses import dataclass -from dbt_score.config import MedalConfig +from dbt_score.config import Config if typing.TYPE_CHECKING: from dbt_score.evaluation import ModelResultsType from dbt_score.rule import RuleViolation, Severity -MAX_SCORE = 10.0 -MIN_SCORE = 0.0 - @dataclass class Score: """Class representing a score.""" score: float - medal: str + badge: str class Scorer: @@ -36,9 +33,9 @@ class Scorer: min_score = 0.0 max_score = 10.0 - def __init__(self, medal_config: MedalConfig) -> None: + def __init__(self, config: Config) -> None: """Create a Scorer object.""" - self._medal_config = medal_config + self._config = config def score_model(self, model_results: ModelResultsType) -> Score: """Compute the score of a given model.""" @@ -67,30 +64,28 @@ def score_model(self, model_results: ModelResultsType) -> Score: * self.max_score ) - return Score(score, self._medal(score)) + return Score(score, self._badge(score)) def score_aggregate_models(self, scores: list[Score]) -> Score: """Compute the score of a list of models.""" actual_scores = [s.score for s in scores] if 0.0 in actual_scores: # Any model with a CRITICAL violation makes the project score 0 - score = Score(self.min_score, self._medal(self.min_score)) + score = Score(self.min_score, self._badge(self.min_score)) elif len(actual_scores) == 0: - score = Score(self.max_score, self._medal(self.max_score)) + score = Score(self.max_score, self._badge(self.max_score)) else: average_score = sum(actual_scores) / len(actual_scores) - score = Score(average_score, self._medal(average_score)) + score = Score(average_score, self._badge(average_score)) return score - def _medal(self, score: float) -> str: - """Compute the medal of a given score.""" - if score >= self._medal_config.gold.threshold: - return self._medal_config.gold.icon - elif score >= self._medal_config.silver.threshold: - return self._medal_config.silver.icon - elif score >= self._medal_config.bronze.threshold: - return self._medal_config.bronze.icon - elif score >= self._medal_config.wip.threshold: - return self._medal_config.wip.icon + def _badge(self, score: float) -> str: + """Compute the badge of a given score.""" + if score >= self._config.badge_config.first.threshold: + return self._config.badge_config.first.icon + elif score >= self._config.badge_config.second.threshold: + return self._config.badge_config.second.icon + elif score >= self._config.badge_config.third.threshold: + return self._config.badge_config.third.icon else: - return "" + return self._config.badge_config.wip_icon diff --git a/tests/formatters/test_manifest_formatter.py b/tests/formatters/test_manifest_formatter.py index 6deb8a6..fa2bd0f 100644 --- a/tests/formatters/test_manifest_formatter.py +++ b/tests/formatters/test_manifest_formatter.py @@ -56,6 +56,6 @@ def test_manifest_formatter_project( stdout = capsys.readouterr().out new_manifest = json.loads(stdout) assert new_manifest["nodes"]["model.package.model1"]["meta"]["score"] == 5.0 - assert new_manifest["nodes"]["model.package.model1"]["meta"]["medal"] == "🚧" + assert new_manifest["nodes"]["model.package.model1"]["meta"]["badge"] == "🚧" assert new_manifest["nodes"]["model.package.model2"]["meta"]["score"] == 10.0 - assert new_manifest["nodes"]["model.package.model2"]["meta"]["medal"] == "🥇" + assert new_manifest["nodes"]["model.package.model2"]["meta"]["badge"] == "🥇" diff --git a/tests/resources/pyproject.toml b/tests/resources/pyproject.toml index dc4d8f2..e4b4e44 100644 --- a/tests/resources/pyproject.toml +++ b/tests/resources/pyproject.toml @@ -2,10 +2,12 @@ rule_namespaces = ["foo", "tests"] disabled_rules = ["foo.foo", "tests.bar"] -[tool.dbt-score.medals] -bronze.threshold = 6.0 +[tool.dbt-score.badges] +wip_icon = "🏗️" +third.threshold = 6.0 +third.icon = "🥉" -[tool.dbt-score.medals.silver] +[tool.dbt-score.badges.second] threshold = 7.0 [tool.dbt-score.rules."foo.bar"] diff --git a/tests/test_config.py b/tests/test_config.py index 3d377d8..1ff9e67 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -2,7 +2,7 @@ from pathlib import Path import pytest -from dbt_score.config import Config, MedalConfig +from dbt_score.config import BadgeConfig, Config from dbt_score.rule import RuleConfig, Severity @@ -17,10 +17,10 @@ def test_load_valid_toml_file(valid_config_path): config.rules_config["tests.rules.example.rule_test_example"].severity == Severity.CRITICAL ) - assert config.medal_config.bronze.threshold == 6.0 - assert config.medal_config.silver.threshold == 7.0 - assert config.medal_config.gold.threshold == 10.0 - assert config.medal_config.wip.threshold == 0.0 + assert config.badge_config.third.threshold == 6.0 + assert config.badge_config.second.threshold == 7.0 + assert config.badge_config.first.threshold == 10.0 + assert config.badge_config.wip_icon == "🏗️" def test_load_invalid_toml_file(caplog, invalid_config_path): @@ -41,21 +41,21 @@ def test_invalid_rule_config(rule_severity_low): rule_severity_low(config) -def test_invalid_medal_thresholds(): - """Test that invalid medal thresholds raises an exception.""" - medal_config = MedalConfig() - medal_config.bronze.threshold = 9.0 - medal_config.silver.threshold = 8.0 - medal_config.gold.threshold = 10.0 - with pytest.raises(ValueError, match="bronze threshold must be lower than"): - medal_config.validate() - - medal_config = MedalConfig() - medal_config.bronze.threshold = 8.0 - medal_config.silver.threshold = 9.5 - medal_config.gold.threshold = 9.5 - with pytest.raises(ValueError, match="silver threshold must be lower than"): - medal_config.validate() +def test_invalid_badge_thresholds(): + """Test that invalid badge thresholds raises an exception.""" + badge_config = BadgeConfig() + badge_config.third.threshold = 9.0 + badge_config.second.threshold = 8.0 + badge_config.first.threshold = 10.0 + with pytest.raises(ValueError, match="third threshold must be lower than"): + badge_config.validate() + + badge_config = BadgeConfig() + badge_config.third.threshold = 8.0 + badge_config.second.threshold = 9.5 + badge_config.first.threshold = 9.5 + with pytest.raises(ValueError, match="second threshold must be lower than"): + badge_config.validate() def test_valid_rule_config(valid_config_path, rule_with_config): diff --git a/tests/test_scoring.py b/tests/test_scoring.py index 46c89c8..2f68570 100644 --- a/tests/test_scoring.py +++ b/tests/test_scoring.py @@ -7,13 +7,13 @@ def test_scorer_model_no_results(default_config): """Test scorer with a model without any result.""" - scorer = Scorer(medal_config=default_config.medal_config) + scorer = Scorer(config=default_config) assert scorer.score_model({}).score == 10.0 def test_scorer_model_severity_low(default_config, rule_severity_low): """Test scorer with a model and one low severity rule.""" - scorer = Scorer(medal_config=default_config.medal_config) + scorer = Scorer(config=default_config) assert scorer.score_model({rule_severity_low: None}).score == 10.0 assert scorer.score_model({rule_severity_low: Exception()}).score == 10.0 assert ( @@ -24,7 +24,7 @@ def test_scorer_model_severity_low(default_config, rule_severity_low): def test_scorer_model_severity_medium(default_config, rule_severity_medium): """Test scorer with a model and one medium severity rule.""" - scorer = Scorer(medal_config=default_config.medal_config) + scorer = Scorer(config=default_config) assert scorer.score_model({rule_severity_medium: None}).score == 10.0 assert scorer.score_model({rule_severity_medium: Exception()}).score == 10.0 assert ( @@ -37,7 +37,7 @@ def test_scorer_model_severity_medium(default_config, rule_severity_medium): def test_scorer_model_severity_high(default_config, rule_severity_high): """Test scorer with a model and one high severity rule.""" - scorer = Scorer(medal_config=default_config.medal_config) + scorer = Scorer(config=default_config) assert scorer.score_model({rule_severity_high: None}).score == 10.0 assert scorer.score_model({rule_severity_high: Exception()}).score == 10.0 assert scorer.score_model({rule_severity_high: RuleViolation("error")}).score == 0.0 @@ -45,7 +45,7 @@ def test_scorer_model_severity_high(default_config, rule_severity_high): def test_scorer_model_severity_critical(default_config, rule_severity_critical): """Test scorer with a model and one critical severity rule.""" - scorer = Scorer(medal_config=default_config.medal_config) + scorer = Scorer(config=default_config) assert scorer.score_model({rule_severity_critical: None}).score == 10.0 assert scorer.score_model({rule_severity_critical: Exception()}).score == 10.0 assert ( @@ -58,7 +58,7 @@ def test_scorer_model_severity_critical_overwrites( default_config, rule_severity_low, rule_severity_critical ): """Test scorer with a model and multiple rules including one critical.""" - scorer = Scorer(medal_config=default_config.medal_config) + scorer = Scorer(config=default_config) assert ( scorer.score_model( {rule_severity_low: None, rule_severity_critical: RuleViolation("error")} @@ -71,7 +71,7 @@ def test_scorer_model_multiple_rules( default_config, rule_severity_low, rule_severity_medium, rule_severity_high ): """Test scorer with a model and multiple rules.""" - scorer = Scorer(medal_config=default_config.medal_config) + scorer = Scorer(config=default_config) assert ( round( scorer.score_model( @@ -117,26 +117,26 @@ def test_scorer_model_multiple_rules( def test_scorer_aggregate_empty(default_config): """Test scorer aggregation with no results.""" - scorer = Scorer(medal_config=default_config.medal_config) + scorer = Scorer(config=default_config) assert scorer.score_aggregate_models([]).score == 10.0 def test_scorer_aggregate_with_0(default_config): """Test scorer aggregation with one result that is 0.0.""" - scorer = Scorer(medal_config=default_config.medal_config) + scorer = Scorer(config=default_config) scores = [Score(1.0, ""), Score(5.0, ""), Score(0.0, "")] assert scorer.score_aggregate_models(scores).score == 0.0 def test_scorer_aggregate_single(default_config): """Test scorer aggregation with a single results.""" - scorer = Scorer(medal_config=default_config.medal_config) + scorer = Scorer(config=default_config) assert scorer.score_aggregate_models([Score(4.2, "")]).score == 4.2 def test_scorer_aggregate_multiple(default_config): """Test scorer aggregation with multiple results.""" - scorer = Scorer(medal_config=default_config.medal_config) + scorer = Scorer(config=default_config) assert ( scorer.score_aggregate_models( [Score(1.0, ""), Score(1.0, ""), Score(1.0, "")] @@ -157,10 +157,10 @@ def test_scorer_aggregate_multiple(default_config): ) -def test_scorer_medal(default_config): - """Test scorer awarding a medal.""" - scorer = Scorer(medal_config=default_config.medal_config) - assert scorer._medal(10.0) == scorer._medal_config.gold.icon - assert scorer._medal(8.0) == scorer._medal_config.silver.icon - assert scorer._medal(7.0) == scorer._medal_config.bronze.icon - assert scorer._medal(1.0) == scorer._medal_config.wip.icon +def test_scorer_badge(default_config): + """Test scorer awarding a badge.""" + scorer = Scorer(config=default_config) + assert scorer._badge(10.0) == scorer._config.badge_config.first.icon + assert scorer._badge(8.0) == scorer._config.badge_config.second.icon + assert scorer._badge(7.0) == scorer._config.badge_config.third.icon + assert scorer._badge(1.0) == scorer._config.badge_config.wip_icon From f70b98ff55f08f3904e0b21a688c21a5e60404ee Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Thu, 30 May 2024 09:26:52 +0200 Subject: [PATCH 10/21] Rename Score.score to Score.value --- .../formatters/human_readable_formatter.py | 4 +- .../formatters/manifest_formatter.py | 2 +- src/dbt_score/scoring.py | 4 +- tests/test_scoring.py | 46 +++++++++---------- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/dbt_score/formatters/human_readable_formatter.py b/src/dbt_score/formatters/human_readable_formatter.py index d6cc5c5..594b0ba 100644 --- a/src/dbt_score/formatters/human_readable_formatter.py +++ b/src/dbt_score/formatters/human_readable_formatter.py @@ -26,7 +26,7 @@ def model_evaluated( ) -> None: """Callback when a model has been evaluated.""" print( - f"{score.badge} {self.bold(model.name)} (score: {round(score.score, 1)!s})" + f"{score.badge} {self.bold(model.name)} (score: {round(score.value, 1)!s})" ) for rule, result in results.items(): if result is None: @@ -42,4 +42,4 @@ def model_evaluated( def project_evaluated(self, score: Score) -> None: """Callback when a project has been evaluated.""" - print(f"Project score: {self.bold(str(round(score.score, 1)))} {score.badge}") + print(f"Project score: {self.bold(str(round(score.value, 1)))} {score.badge}") diff --git a/src/dbt_score/formatters/manifest_formatter.py b/src/dbt_score/formatters/manifest_formatter.py index 01c3862..a1914cb 100644 --- a/src/dbt_score/formatters/manifest_formatter.py +++ b/src/dbt_score/formatters/manifest_formatter.py @@ -28,6 +28,6 @@ def project_evaluated(self, score: Score) -> None: """Callback when a project has been evaluated.""" manifest = copy.copy(self._manifest_loader.raw_manifest) for model_id, model_score in self._model_scores.items(): - manifest["nodes"][model_id]["meta"]["score"] = model_score.score + manifest["nodes"][model_id]["meta"]["score"] = model_score.value manifest["nodes"][model_id]["meta"]["badge"] = model_score.badge print(json.dumps(manifest, indent=2)) diff --git a/src/dbt_score/scoring.py b/src/dbt_score/scoring.py index eda9e30..81a1554 100644 --- a/src/dbt_score/scoring.py +++ b/src/dbt_score/scoring.py @@ -16,7 +16,7 @@ class Score: """Class representing a score.""" - score: float + value: float badge: str @@ -68,7 +68,7 @@ def score_model(self, model_results: ModelResultsType) -> Score: def score_aggregate_models(self, scores: list[Score]) -> Score: """Compute the score of a list of models.""" - actual_scores = [s.score for s in scores] + actual_scores = [s.value for s in scores] if 0.0 in actual_scores: # Any model with a CRITICAL violation makes the project score 0 score = Score(self.min_score, self._badge(self.min_score)) diff --git a/tests/test_scoring.py b/tests/test_scoring.py index 2f68570..a33f49e 100644 --- a/tests/test_scoring.py +++ b/tests/test_scoring.py @@ -8,16 +8,16 @@ def test_scorer_model_no_results(default_config): """Test scorer with a model without any result.""" scorer = Scorer(config=default_config) - assert scorer.score_model({}).score == 10.0 + assert scorer.score_model({}).value == 10.0 def test_scorer_model_severity_low(default_config, rule_severity_low): """Test scorer with a model and one low severity rule.""" scorer = Scorer(config=default_config) - assert scorer.score_model({rule_severity_low: None}).score == 10.0 - assert scorer.score_model({rule_severity_low: Exception()}).score == 10.0 + assert scorer.score_model({rule_severity_low: None}).value == 10.0 + assert scorer.score_model({rule_severity_low: Exception()}).value == 10.0 assert ( - round(scorer.score_model({rule_severity_low: RuleViolation("error")}).score, 2) + round(scorer.score_model({rule_severity_low: RuleViolation("error")}).value, 2) == 6.67 ) @@ -25,11 +25,11 @@ def test_scorer_model_severity_low(default_config, rule_severity_low): def test_scorer_model_severity_medium(default_config, rule_severity_medium): """Test scorer with a model and one medium severity rule.""" scorer = Scorer(config=default_config) - assert scorer.score_model({rule_severity_medium: None}).score == 10.0 - assert scorer.score_model({rule_severity_medium: Exception()}).score == 10.0 + assert scorer.score_model({rule_severity_medium: None}).value == 10.0 + assert scorer.score_model({rule_severity_medium: Exception()}).value == 10.0 assert ( round( - scorer.score_model({rule_severity_medium: RuleViolation("error")}).score, 2 + scorer.score_model({rule_severity_medium: RuleViolation("error")}).value, 2 ) == 3.33 ) @@ -38,18 +38,18 @@ def test_scorer_model_severity_medium(default_config, rule_severity_medium): def test_scorer_model_severity_high(default_config, rule_severity_high): """Test scorer with a model and one high severity rule.""" scorer = Scorer(config=default_config) - assert scorer.score_model({rule_severity_high: None}).score == 10.0 - assert scorer.score_model({rule_severity_high: Exception()}).score == 10.0 - assert scorer.score_model({rule_severity_high: RuleViolation("error")}).score == 0.0 + assert scorer.score_model({rule_severity_high: None}).value == 10.0 + assert scorer.score_model({rule_severity_high: Exception()}).value == 10.0 + assert scorer.score_model({rule_severity_high: RuleViolation("error")}).value == 0.0 def test_scorer_model_severity_critical(default_config, rule_severity_critical): """Test scorer with a model and one critical severity rule.""" scorer = Scorer(config=default_config) - assert scorer.score_model({rule_severity_critical: None}).score == 10.0 - assert scorer.score_model({rule_severity_critical: Exception()}).score == 10.0 + assert scorer.score_model({rule_severity_critical: None}).value == 10.0 + assert scorer.score_model({rule_severity_critical: Exception()}).value == 10.0 assert ( - scorer.score_model({rule_severity_critical: RuleViolation("error")}).score + scorer.score_model({rule_severity_critical: RuleViolation("error")}).value == 0.0 ) @@ -62,7 +62,7 @@ def test_scorer_model_severity_critical_overwrites( assert ( scorer.score_model( {rule_severity_low: None, rule_severity_critical: RuleViolation("error")} - ).score + ).value == 0.0 ) @@ -80,7 +80,7 @@ def test_scorer_model_multiple_rules( rule_severity_medium: Exception(), rule_severity_high: RuleViolation("error"), } - ).score, + ).value, 2, ) == 6.67 @@ -94,7 +94,7 @@ def test_scorer_model_multiple_rules( rule_severity_medium: RuleViolation("error"), rule_severity_high: None, } - ).score, + ).value, 2, ) == 7.78 @@ -108,7 +108,7 @@ def test_scorer_model_multiple_rules( rule_severity_medium: Exception(), rule_severity_high: None, } - ).score, + ).value, 2, ) == 8.89 @@ -118,20 +118,20 @@ def test_scorer_model_multiple_rules( def test_scorer_aggregate_empty(default_config): """Test scorer aggregation with no results.""" scorer = Scorer(config=default_config) - assert scorer.score_aggregate_models([]).score == 10.0 + assert scorer.score_aggregate_models([]).value == 10.0 def test_scorer_aggregate_with_0(default_config): """Test scorer aggregation with one result that is 0.0.""" scorer = Scorer(config=default_config) scores = [Score(1.0, ""), Score(5.0, ""), Score(0.0, "")] - assert scorer.score_aggregate_models(scores).score == 0.0 + assert scorer.score_aggregate_models(scores).value == 0.0 def test_scorer_aggregate_single(default_config): """Test scorer aggregation with a single results.""" scorer = Scorer(config=default_config) - assert scorer.score_aggregate_models([Score(4.2, "")]).score == 4.2 + assert scorer.score_aggregate_models([Score(4.2, "")]).value == 4.2 def test_scorer_aggregate_multiple(default_config): @@ -140,19 +140,19 @@ def test_scorer_aggregate_multiple(default_config): assert ( scorer.score_aggregate_models( [Score(1.0, ""), Score(1.0, ""), Score(1.0, "")] - ).score + ).value == 1.0 ) assert ( scorer.score_aggregate_models( [Score(1.0, ""), Score(7.4, ""), Score(4.2, "")] - ).score + ).value == 4.2 ) assert ( scorer.score_aggregate_models( [Score(0.0, ""), Score(0.0, ""), Score(0.0, "")] - ).score + ).value == 0.0 ) From 1d78f56cadd369188f4223d518efc418ed3da092 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Thu, 30 May 2024 09:28:37 +0200 Subject: [PATCH 11/21] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6384542..64aa500 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,3 +17,4 @@ and this project adheres to - Linting and scoring functionality for dbt models. - Configuration through `pyproject.toml`. - Default rules in `dbt_score.rules.generic`. +- Badges for project and model evaluation. From 32f4f68c6f619da24b2bc8ff3dfc3f97e8cd050b Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Thu, 30 May 2024 13:35:04 +0200 Subject: [PATCH 12/21] Make wip a Badge --- src/dbt_score/config.py | 11 ++++++----- src/dbt_score/scoring.py | 2 +- tests/resources/pyproject.toml | 13 +++++++++---- tests/test_config.py | 11 +++++++---- tests/test_scoring.py | 2 +- 5 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/dbt_score/config.py b/src/dbt_score/config.py index b51fb34..18be90f 100644 --- a/src/dbt_score/config.py +++ b/src/dbt_score/config.py @@ -28,7 +28,7 @@ class BadgeConfig: third: Badge = field(default_factory=lambda: Badge("🥉", 6.0)) second: Badge = field(default_factory=lambda: Badge("🥈", 8.0)) first: Badge = field(default_factory=lambda: Badge("🥇", 10.0)) - wip_icon: str = "🚧" + wip: Badge = field(default_factory=lambda: Badge("🚧", 0.0)) @classmethod def load_from_dict(cls, badge_config: dict[str, Any]) -> "BadgeConfig": @@ -42,10 +42,11 @@ def load_from_dict(cls, badge_config: dict[str, Any]) -> "BadgeConfig": badge_defaults = asdict(default_badge_config.__getattribute__(badge)) badge_defaults.update(badge_options) options[badge] = Badge(**badge_defaults) - # The wip icon is not a dictionary - # and is not a Badge object - elif badge == "wip_icon": - options[badge] = badge_options + + if badge == "wip" and badge_options.get("threshold"): + raise AttributeError( + "wip badge cannot have a threshold configuration." + ) else: raise AttributeError( f"Invalid config for badge: {badge}, must be a dictionary." diff --git a/src/dbt_score/scoring.py b/src/dbt_score/scoring.py index 81a1554..abf9363 100644 --- a/src/dbt_score/scoring.py +++ b/src/dbt_score/scoring.py @@ -88,4 +88,4 @@ def _badge(self, score: float) -> str: elif score >= self._config.badge_config.third.threshold: return self._config.badge_config.third.icon else: - return self._config.badge_config.wip_icon + return self._config.badge_config.wip.icon diff --git a/tests/resources/pyproject.toml b/tests/resources/pyproject.toml index e4b4e44..b317d90 100644 --- a/tests/resources/pyproject.toml +++ b/tests/resources/pyproject.toml @@ -3,12 +3,17 @@ rule_namespaces = ["foo", "tests"] disabled_rules = ["foo.foo", "tests.bar"] [tool.dbt-score.badges] -wip_icon = "🏗️" -third.threshold = 6.0 -third.icon = "🥉" +wip.icon = "🏗️" +third.threshold = 6.5 +third.icon = "3️⃣" [tool.dbt-score.badges.second] -threshold = 7.0 +threshold = 7.5 +icon = "2️⃣" + +[tool.dbt-score.badges.first] +threshold = 9.5 +icon = "1️⃣" [tool.dbt-score.rules."foo.bar"] severity=4 diff --git a/tests/test_config.py b/tests/test_config.py index 1ff9e67..04744bc 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -17,10 +17,13 @@ def test_load_valid_toml_file(valid_config_path): config.rules_config["tests.rules.example.rule_test_example"].severity == Severity.CRITICAL ) - assert config.badge_config.third.threshold == 6.0 - assert config.badge_config.second.threshold == 7.0 - assert config.badge_config.first.threshold == 10.0 - assert config.badge_config.wip_icon == "🏗️" + assert config.badge_config.third.threshold == 6.5 + assert config.badge_config.second.threshold == 7.5 + assert config.badge_config.first.threshold == 9.5 + assert config.badge_config.wip.icon == "🏗️" + assert config.badge_config.third.icon == "3️⃣" + assert config.badge_config.second.icon == "2️⃣" + assert config.badge_config.first.icon == "1️⃣" def test_load_invalid_toml_file(caplog, invalid_config_path): diff --git a/tests/test_scoring.py b/tests/test_scoring.py index a33f49e..e47a493 100644 --- a/tests/test_scoring.py +++ b/tests/test_scoring.py @@ -163,4 +163,4 @@ def test_scorer_badge(default_config): assert scorer._badge(10.0) == scorer._config.badge_config.first.icon assert scorer._badge(8.0) == scorer._config.badge_config.second.icon assert scorer._badge(7.0) == scorer._config.badge_config.third.icon - assert scorer._badge(1.0) == scorer._config.badge_config.wip_icon + assert scorer._badge(1.0) == scorer._config.badge_config.wip.icon From 3349c3321e6f89dc809c2eb6b9c3f413d695aed5 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Thu, 30 May 2024 14:52:43 +0200 Subject: [PATCH 13/21] Document configuration of badges --- docs/configuration.md | 21 ++++++++++ docs/index.md | 7 ++-- docs/rules/generic.md | 96 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 120 insertions(+), 4 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 7d00b0d..0ca9112 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -38,6 +38,27 @@ The following options can be set in the `pyproject.toml` file: if not included here. - `disabled_rules`: A list of rules to disable. +#### Badges configuration + +```toml +[tool.dbt-score.badges] +``` + +Four badges can be configured: `first`, `second`, `third` and `wip`. Each badge +can be configured with the following option: + +- `icon`: The icon to use for the badge. A string that will be displayed in the + output, e.g. `🥇`. + +All badges except `wip` can be configured with the following option: + +- `threshold`: The threshold for the badge. A float that will be used to compare + to the score. The threshold is the minimum score required for a model to be + rewarded with a certain badge. + +The default values can be found in the +[BadgeConfig](/reference/config/#dbt_score.config.BadgeConfig). + #### Rule configuration ```toml diff --git a/docs/index.md b/docs/index.md index da15bd3..a95b621 100644 --- a/docs/index.md +++ b/docs/index.md @@ -12,15 +12,16 @@ encourage) good practices. ```shell $ dbt-score lint -Model customers +🥉 customers (score: 6.7) OK dbt_score.rules.generic.has_description WARN (medium) dbt_score.rules.generic.has_owner: Model lacks an owner. OK dbt_score.rules.generic.sql_has_reasonable_number_of_lines -Score: 6.7 +Score: 6.7 🥉 ``` In this example, `dbt-score` reports a warning: the model `customers` does not -declare an owner. Hence, it doesn't score the maximum value of `10`. +declare an owner. Hence, it doesn't score the maximum value of `10`. It also is +awarded a bronze medal because of the score. ## Philosophy diff --git a/docs/rules/generic.md b/docs/rules/generic.md index 504a607..2823823 100644 --- a/docs/rules/generic.md +++ b/docs/rules/generic.md @@ -1 +1,95 @@ -(content generated in CI) +# Generic + +## `columns_have_description` + +All columns of a model should have a description. + +??? quote "Source code" ```python @rule def columns_have_description(model: +Model) -> RuleViolation | None: """All columns of a model should have a +description.""" invalid_column_names = [ column.name for column in model.columns +if not column.description ] if invalid_column_names: max_length = 60 message = +f"Columns lack a description: {', '.join(invalid_column_names)}." if +len(message) > max_length: message = f"{message[:60]}…" return +RuleViolation(message=message) + + ``` + +### Default configuration + +```toml title="pyproject.toml" +[tool.dbt-score.rules."dbt_score.rules.generic.columns_have_description"] +severity = 2 +``` + +## `has_description` + +A model should have a description. + +??? quote "Source code" ```python @rule def has_description(model: Model) -> +RuleViolation | None: """A model should have a description.""" if not +model.description: return RuleViolation(message="Model lacks a description.") + + ``` + +### Default configuration + +```toml title="pyproject.toml" +[tool.dbt-score.rules."dbt_score.rules.generic.has_description"] +severity = 2 +``` + +## `has_owner` + +A model should have an owner. + +??? quote "Source code" ```python @rule def has_owner(model: Model) -> +RuleViolation | None: """A model should have an owner.""" if not +model.meta.get("owner"): return RuleViolation(message="Model lacks an owner.") + + ``` + +### Default configuration + +```toml title="pyproject.toml" +[tool.dbt-score.rules."dbt_score.rules.generic.has_owner"] +severity = 2 +``` + +## `public_model_has_example_sql` + +The documentation of a public model should have an example query. + +??? quote "Source code" +`python @rule(severity=Severity.LOW) def public_model_has_example_sql(model: Model) -> RuleViolation | None: """The documentation of a public model should have an example query.""" if model.language == "sql" and model.access == "public": if "`sql" +not in model.description: return RuleViolation( "The model description does not +include an example SQL query." ) + + ``` + +### Default configuration + +```toml title="pyproject.toml" +[tool.dbt-score.rules."dbt_score.rules.generic.public_model_has_example_sql"] +severity = 1 +``` + +## `sql_has_reasonable_number_of_lines` + +The SQL query of a model should not be too long. + +??? quote "Source code" ```python @rule def sql_has_reasonable_number_of_lines( +model: Model, max_lines: int = 200 ) -> RuleViolation | None: """The SQL query +of a model should not be too long.""" count_lines = +len(model.raw_code.split("\n")) if count_lines > max_lines: return +RuleViolation( message=f"SQL query too long: {count_lines} lines (> +{max_lines})." ) + + ``` + +### Default configuration + +```toml title="pyproject.toml" +[tool.dbt-score.rules."dbt_score.rules.generic.sql_has_reasonable_number_of_lines"] +severity = 1 +max_lines = 200 +``` From 5ce5ca06f229aca6317605b66fbea0ea6f754f61 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Thu, 30 May 2024 14:57:45 +0200 Subject: [PATCH 14/21] Improve test for invalid medal config --- tests/test_config.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_config.py b/tests/test_config.py index 04744bc..04ec9ab 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -60,6 +60,12 @@ def test_invalid_badge_thresholds(): with pytest.raises(ValueError, match="second threshold must be lower than"): badge_config.validate() + badge_config = BadgeConfig() + with pytest.raises( + AttributeError, match="wip badge cannot have a threshold configuration." + ): + badge_config.load_from_dict({"wip": {"threshold": 10.0}}) + def test_valid_rule_config(valid_config_path, rule_with_config): """Test that a valid rule config can be loaded.""" From 7325a34cb14d42bc89d094b09f6d3b0f4f6ed8a9 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Thu, 30 May 2024 15:06:35 +0200 Subject: [PATCH 15/21] Improve documentation --- docs/configuration.md | 15 +++++-- docs/index.md | 12 +++--- docs/rules/generic.md | 96 +------------------------------------------ 3 files changed, 19 insertions(+), 104 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 0ca9112..861679a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -20,6 +20,15 @@ disabled_rules = ["dbt_score.rules.generic.columns_have_description"] [tool.dbt-score.rules."dbt_score.rules.generic.sql_has_reasonable_number_of_lines"] severity = 1 max_lines = 300 + +[tool.dbt-score.badges] +first.threshold = 9.5 +first.icon = "1️⃣" +second.threshold = 7.5 +second.icon = "2️⃣" +third.threshold = 6.5 +third.icon = "3️⃣" +wip.icon = "🏗️" ``` ### Configuration options @@ -52,9 +61,9 @@ can be configured with the following option: All badges except `wip` can be configured with the following option: -- `threshold`: The threshold for the badge. A float that will be used to compare - to the score. The threshold is the minimum score required for a model to be - rewarded with a certain badge. +- `threshold`: The threshold for the badge. A float between `0.0` and `10.0` + that will be used to compare to the score. The threshold is the minimum score + required for a model to be rewarded with a certain badge. The default values can be found in the [BadgeConfig](/reference/config/#dbt_score.config.BadgeConfig). diff --git a/docs/index.md b/docs/index.md index a95b621..dd90e8a 100644 --- a/docs/index.md +++ b/docs/index.md @@ -12,16 +12,16 @@ encourage) good practices. ```shell $ dbt-score lint -🥉 customers (score: 6.7) +🥇 customers (score: 10.0) OK dbt_score.rules.generic.has_description - WARN (medium) dbt_score.rules.generic.has_owner: Model lacks an owner. + OK dbt_score.rules.generic.has_owner: Model lacks an owner. OK dbt_score.rules.generic.sql_has_reasonable_number_of_lines -Score: 6.7 🥉 +Score: 10.0 🥇 ``` -In this example, `dbt-score` reports a warning: the model `customers` does not -declare an owner. Hence, it doesn't score the maximum value of `10`. It also is -awarded a bronze medal because of the score. +In this example, the model `customers` score the maximum value of `10.0` as it +passes all the rules. It also is awarded a golden medal because of the perfect +score. ## Philosophy diff --git a/docs/rules/generic.md b/docs/rules/generic.md index 2823823..504a607 100644 --- a/docs/rules/generic.md +++ b/docs/rules/generic.md @@ -1,95 +1 @@ -# Generic - -## `columns_have_description` - -All columns of a model should have a description. - -??? quote "Source code" ```python @rule def columns_have_description(model: -Model) -> RuleViolation | None: """All columns of a model should have a -description.""" invalid_column_names = [ column.name for column in model.columns -if not column.description ] if invalid_column_names: max_length = 60 message = -f"Columns lack a description: {', '.join(invalid_column_names)}." if -len(message) > max_length: message = f"{message[:60]}…" return -RuleViolation(message=message) - - ``` - -### Default configuration - -```toml title="pyproject.toml" -[tool.dbt-score.rules."dbt_score.rules.generic.columns_have_description"] -severity = 2 -``` - -## `has_description` - -A model should have a description. - -??? quote "Source code" ```python @rule def has_description(model: Model) -> -RuleViolation | None: """A model should have a description.""" if not -model.description: return RuleViolation(message="Model lacks a description.") - - ``` - -### Default configuration - -```toml title="pyproject.toml" -[tool.dbt-score.rules."dbt_score.rules.generic.has_description"] -severity = 2 -``` - -## `has_owner` - -A model should have an owner. - -??? quote "Source code" ```python @rule def has_owner(model: Model) -> -RuleViolation | None: """A model should have an owner.""" if not -model.meta.get("owner"): return RuleViolation(message="Model lacks an owner.") - - ``` - -### Default configuration - -```toml title="pyproject.toml" -[tool.dbt-score.rules."dbt_score.rules.generic.has_owner"] -severity = 2 -``` - -## `public_model_has_example_sql` - -The documentation of a public model should have an example query. - -??? quote "Source code" -`python @rule(severity=Severity.LOW) def public_model_has_example_sql(model: Model) -> RuleViolation | None: """The documentation of a public model should have an example query.""" if model.language == "sql" and model.access == "public": if "`sql" -not in model.description: return RuleViolation( "The model description does not -include an example SQL query." ) - - ``` - -### Default configuration - -```toml title="pyproject.toml" -[tool.dbt-score.rules."dbt_score.rules.generic.public_model_has_example_sql"] -severity = 1 -``` - -## `sql_has_reasonable_number_of_lines` - -The SQL query of a model should not be too long. - -??? quote "Source code" ```python @rule def sql_has_reasonable_number_of_lines( -model: Model, max_lines: int = 200 ) -> RuleViolation | None: """The SQL query -of a model should not be too long.""" count_lines = -len(model.raw_code.split("\n")) if count_lines > max_lines: return -RuleViolation( message=f"SQL query too long: {count_lines} lines (> -{max_lines})." ) - - ``` - -### Default configuration - -```toml title="pyproject.toml" -[tool.dbt-score.rules."dbt_score.rules.generic.sql_has_reasonable_number_of_lines"] -severity = 1 -max_lines = 200 -``` +(content generated in CI) From 80653e5e571a26d01374f802a3d72fae230a927c Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Thu, 30 May 2024 15:07:33 +0200 Subject: [PATCH 16/21] fix typo --- docs/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/index.md b/docs/index.md index dd90e8a..043a536 100644 --- a/docs/index.md +++ b/docs/index.md @@ -19,7 +19,7 @@ $ dbt-score lint Score: 10.0 🥇 ``` -In this example, the model `customers` score the maximum value of `10.0` as it +In this example, the model `customers` scores the maximum value of `10.0` as it passes all the rules. It also is awarded a golden medal because of the perfect score. From 34fa1bbe1f0ef7e6807fcd2de2083145d0db3114 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Fri, 31 May 2024 08:49:01 +0200 Subject: [PATCH 17/21] Set medal config example to defaults --- docs/configuration.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 861679a..77a2c3e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -17,18 +17,18 @@ below: rule_namespaces = ["dbt_score.rules", "dbt_score_rules", "custom_rules"] disabled_rules = ["dbt_score.rules.generic.columns_have_description"] +[tool.dbt-score.badges] +first.threshold = 10.0 +first.icon = "🥇" +second.threshold = 8.0 +second.icon = "🥈" +third.threshold = 6.0 +third.icon = "🥉" +wip.icon = "🏗️" + [tool.dbt-score.rules."dbt_score.rules.generic.sql_has_reasonable_number_of_lines"] severity = 1 max_lines = 300 - -[tool.dbt-score.badges] -first.threshold = 9.5 -first.icon = "1️⃣" -second.threshold = 7.5 -second.icon = "2️⃣" -third.threshold = 6.5 -third.icon = "3️⃣" -wip.icon = "🏗️" ``` ### Configuration options From 9385ffba93bc8b01eee07a5e95a1770b96f19912 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Mon, 3 Jun 2024 09:29:47 +0200 Subject: [PATCH 18/21] Make changes to medals and add tests --- src/dbt_score/config.py | 60 +++++++++++++++++------------------------ tests/test_config.py | 45 ++++++++++++++++++++++++++++--- 2 files changed, 66 insertions(+), 39 deletions(-) diff --git a/src/dbt_score/config.py b/src/dbt_score/config.py index 18be90f..f15e2a9 100644 --- a/src/dbt_score/config.py +++ b/src/dbt_score/config.py @@ -2,7 +2,7 @@ import logging import tomllib -from dataclasses import asdict, dataclass, field +from dataclasses import dataclass, field, replace from pathlib import Path from typing import Any, Final @@ -30,43 +30,16 @@ class BadgeConfig: first: Badge = field(default_factory=lambda: Badge("🥇", 10.0)) wip: Badge = field(default_factory=lambda: Badge("🚧", 0.0)) - @classmethod - def load_from_dict(cls, badge_config: dict[str, Any]) -> "BadgeConfig": - """Create a BadgeConfig from a dictionary.""" - options: dict[str, Any] = {} - default_badge_config = cls() - for badge, badge_options in badge_config.items(): - if badge not in default_badge_config.__dataclass_fields__: - raise AttributeError(f"Unknown badge: {badge}.") - if isinstance(badge_options, dict): - badge_defaults = asdict(default_badge_config.__getattribute__(badge)) - badge_defaults.update(badge_options) - options[badge] = Badge(**badge_defaults) - - if badge == "wip" and badge_options.get("threshold"): - raise AttributeError( - "wip badge cannot have a threshold configuration." - ) - else: - raise AttributeError( - f"Invalid config for badge: {badge}, must be a dictionary." - ) - - config = cls(**options) - config.validate() - - return config - def validate(self) -> None: """Validate the badge configuration.""" - if self.third.threshold >= self.second.threshold: - raise ValueError("third threshold must be lower than second threshold") - if self.second.threshold >= self.first.threshold: - raise ValueError("second threshold must be lower than first threshold") + if not (self.first.threshold > self.second.threshold > self.third.threshold): + raise ValueError("Invalid badge thresholds.") if self.first.threshold > 10.0: # noqa: PLR2004 [magic-value-comparison] - raise ValueError("first threshold must 10.0 or lower") + raise ValueError("first threshold must 10.0 or lower.") if self.third.threshold < 0.0: - raise ValueError("third threshold must be 0.0 or higher") + raise ValueError("third threshold must be 0.0 or higher.") + if self.wip.threshold != 0.0: + raise AttributeError("wip badge cannot have a threshold configuration.") class Config: @@ -114,8 +87,23 @@ def _load_toml_file(self, file: str) -> None: ) # Badge configuration - if badge_config: - self.badge_config = self.badge_config.load_from_dict(badge_config) + for name, config in badge_config.items(): + try: + default_config = getattr(self.badge_config, name) + updated_config = replace(default_config, **config) + setattr(self.badge_config, name, updated_config) + except AttributeError as e: + options = list(BadgeConfig.__annotations__.keys()) + raise AttributeError(f"Config only accepts badges: {options}.") from e + except TypeError as e: + options = list(Badge.__annotations__.keys()) + if name == "wip": + options.remove("threshold") + raise AttributeError( + f"Badge {name}: config only accepts {options}." + ) from e + + self.badge_config.validate() # Rule configuration self.rules_config = { diff --git a/tests/test_config.py b/tests/test_config.py index 04ec9ab..37d93f3 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,5 +1,6 @@ """Tests for the module config_parser.""" from pathlib import Path +from unittest.mock import patch import pytest from dbt_score.config import BadgeConfig, Config @@ -44,27 +45,65 @@ def test_invalid_rule_config(rule_severity_low): rule_severity_low(config) +@patch("dbt_score.config.open") +def test_load_invalid_badge_config(mock_open): + """Test that an invalid badge config raises an exception.""" + config = Config() + + with patch("dbt_score.config.tomllib.load") as mock_load: + mock_load.return_value = { + "tool": {"dbt-score": {"badges": {"wip": {"threshold": 1.0}}}} + } + with pytest.raises( + AttributeError, match="wip badge cannot have a threshold configuration." + ): + config._load_toml_file("foo") + + mock_load.return_value = { + "tool": {"dbt-score": {"badges": {"foo": {"threshold": 1.0}}}} + } + with pytest.raises(AttributeError, match="Config only accepts badges:"): + config._load_toml_file("foo") + + mock_load.return_value = { + "tool": {"dbt-score": {"badges": {"first": {"foo": "bar"}}}} + } + with pytest.raises(AttributeError, match="Badge first: config only accepts"): + config._load_toml_file("foo") + + def test_invalid_badge_thresholds(): """Test that invalid badge thresholds raises an exception.""" badge_config = BadgeConfig() badge_config.third.threshold = 9.0 badge_config.second.threshold = 8.0 badge_config.first.threshold = 10.0 - with pytest.raises(ValueError, match="third threshold must be lower than"): + with pytest.raises(ValueError, match="Invalid badge thresholds."): badge_config.validate() badge_config = BadgeConfig() badge_config.third.threshold = 8.0 badge_config.second.threshold = 9.5 badge_config.first.threshold = 9.5 - with pytest.raises(ValueError, match="second threshold must be lower than"): + with pytest.raises(ValueError, match="Invalid badge thresholds."): badge_config.validate() badge_config = BadgeConfig() + badge_config.third.threshold = -1 + with pytest.raises(ValueError, match="third threshold must be 0.0 or higher."): + badge_config.validate() + + badge_config = BadgeConfig() + badge_config.first.threshold = 11.0 + with pytest.raises(ValueError, match="first threshold must 10.0 or lower."): + badge_config.validate() + + badge_config = BadgeConfig() + badge_config.wip.threshold = 1.0 with pytest.raises( AttributeError, match="wip badge cannot have a threshold configuration." ): - badge_config.load_from_dict({"wip": {"threshold": 10.0}}) + badge_config.validate() def test_valid_rule_config(valid_config_path, rule_with_config): From 252e20dd2600e102c85c5336b918f7ed97dd978e Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Mon, 3 Jun 2024 09:36:46 +0200 Subject: [PATCH 19/21] Remove redundant test --- tests/test_config.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 37d93f3..d70ad73 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -51,14 +51,6 @@ def test_load_invalid_badge_config(mock_open): config = Config() with patch("dbt_score.config.tomllib.load") as mock_load: - mock_load.return_value = { - "tool": {"dbt-score": {"badges": {"wip": {"threshold": 1.0}}}} - } - with pytest.raises( - AttributeError, match="wip badge cannot have a threshold configuration." - ): - config._load_toml_file("foo") - mock_load.return_value = { "tool": {"dbt-score": {"badges": {"foo": {"threshold": 1.0}}}} } From 7c2f027dc91c68df52f1ca3e1832ec9115a2f2ac Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Mon, 3 Jun 2024 10:50:33 +0200 Subject: [PATCH 20/21] Update docs/configuration.md Co-authored-by: Matthieu Caneill --- docs/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration.md b/docs/configuration.md index 77a2c3e..543024b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -61,7 +61,7 @@ can be configured with the following option: All badges except `wip` can be configured with the following option: -- `threshold`: The threshold for the badge. A float between `0.0` and `10.0` +- `threshold`: The threshold for the badge. A decimal number between `0.0` and `10.0` that will be used to compare to the score. The threshold is the minimum score required for a model to be rewarded with a certain badge. From de52b75845446e614414c09a804854474387ec30 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Mon, 3 Jun 2024 10:51:20 +0200 Subject: [PATCH 21/21] Prettier configuration --- docs/configuration.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 543024b..e7fb6f6 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -61,9 +61,9 @@ can be configured with the following option: All badges except `wip` can be configured with the following option: -- `threshold`: The threshold for the badge. A decimal number between `0.0` and `10.0` - that will be used to compare to the score. The threshold is the minimum score - required for a model to be rewarded with a certain badge. +- `threshold`: The threshold for the badge. A decimal number between `0.0` and + `10.0` that will be used to compare to the score. The threshold is the minimum + score required for a model to be rewarded with a certain badge. The default values can be found in the [BadgeConfig](/reference/config/#dbt_score.config.BadgeConfig).