Add documentation

docs/configuration.md

@@ -88,7 +88,7 @@ Every rule can be configured with the following option:
   of 4.
 - `model_filter_names`: Filters used by the rule. Takes a list of names that can
   be found in the same namespace as the rules (see
-  [Package rules](package_rules.md))
+  [Package rules](package_rules.md)).
 
 Some rules have additional configuration options, e.g.
 [sql_has_reasonable_number_of_lines](rules/generic.md#sql_has_reasonable_number_of_lines).

docs/create_rules.md

@@ -39,23 +39,6 @@ from dbt_score import rule, Severity
 @rule(severity=Severity.HIGH)
 ```
 
-If a custom rule should be applied to only a subset of the models in the
-project, a special value of type `SkipRule` can be returned. Models that were
-skipped will be ignored in the final score. More complex uses of skipping rules
-can use Model Filters (see below).
-
-```python
-from dbt_score import rule, Model, RuleViolation, SkipRule
-
-@rule
-def mart_schema_has_description(model: Model) -> RuleViolation | SkipRule | None:
-    """A model in the DataMart should have a description."""
-    if model.schema != 'mart':
-        return SkipRule()
-    if not model.description:
-        return RuleViolation(message="Model lacks a description.")
-```
-
 ### Using the `Rule` class
 
 For more advanced use cases, a rule can be created by inheriting from the `Rule`
@@ -110,13 +93,13 @@ def sql_has_reasonable_number_of_lines(model: Model, max_lines: int = 200) -> Ru
 
 ### Filtering models
 
-Custom and standard rules can be configured to have model filters. Filters
-allows setting models of the project to be ignored by a given rule.
+Custom and standard rules can be configured to have model filters. Filters allow
+models to be ignored by one or multiple rules.
 
 Filters are created using the same discovery mechanism and interface as custom
 rules, except they do not accept parameters. Similar to Python's built-in
 `filter` function, when the filter evaluation returns `True` the model should be
-evaluated, otherwise it should be skipped.
+evaluated, otherwise it should be ignored.
 
 ```python
 from dbt_score import ModelFilter, model_filter
@@ -137,11 +120,11 @@ Similar to setting a rule severity, standard rules can have filters set in the
 while custom rules accept the configuration file or a decorator parameter.
 
 ```python
-from dbt_score import Model, rule, RuleViolation, SkipRule
+from dbt_score import Model, rule, RuleViolation
 from my_project import only_schema_x
 
 @rule(model_filters={only_schema_x()})
-def models_in_x_follow_naming_standard(model: Model) -> RuleViolation | SkipRule | None:
+def models_in_x_follow_naming_standard(model: Model) -> RuleViolation | None:
     """Models in schema X must follow the naming standard."""
     if some_regex_fails(model.name):
         return RuleViolation("Invalid model name.")

src/dbt_score/__init__.py

@@ -2,15 +2,14 @@
 
 from dbt_score.model_filter import ModelFilter, model_filter
 from dbt_score.models import Model
-from dbt_score.rule import Rule, RuleViolation, Severity, SkipRule, rule
+from dbt_score.rule import Rule, RuleViolation, Severity, rule
 
 __all__ = [
     "Model",
     "ModelFilter",
     "Rule",
     "RuleViolation",
     "Severity",
-    "SkipRule",
     "model_filter",
     "rule",
 ]

src/dbt_score/evaluation.py

@@ -6,16 +6,15 @@
 
 from dbt_score.formatters import Formatter
 from dbt_score.models import ManifestLoader, Model
-from dbt_score.rule import Rule, RuleViolation, SkipRule
+from dbt_score.rule import Rule, RuleViolation
 from dbt_score.rule_registry import RuleRegistry
 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
 # - A RuleViolation if a linting error was found
-# - A SkipRule if the rule doesn't apply to the model
 # - An Exception if the rule failed to run
-ModelResultsType = dict[Type[Rule], None | RuleViolation | SkipRule | Exception]
+ModelResultsType = dict[Type[Rule], None | RuleViolation | Exception]
 
 
 class Evaluation:
@@ -58,15 +57,11 @@ def evaluate(self) -> None:
             self.results[model] = {}
             for rule in rules:
                 try:
-                    result: RuleViolation | SkipRule | None = None
-                    if rule.should_evaluate(model):
+                    if rule.should_evaluate(model):  #  Consider model filter(s).
                         result = rule.evaluate(model, **rule.config)
-                    else:
-                        result = SkipRule()
+                        self.results[model][rule.__class__] = result
                 except Exception as e:
                     self.results[model][rule.__class__] = e
-                else:
-                    self.results[model][rule.__class__] = result
 
             self.scores[model] = self._scorer.score_model(self.results[model])
             self._formatter.model_evaluated(

src/dbt_score/formatters/human_readable_formatter.py

@@ -6,7 +6,7 @@
 from dbt_score.evaluation import ModelResultsType
 from dbt_score.formatters import Formatter
 from dbt_score.models import Model
-from dbt_score.rule import RuleViolation, SkipRule
+from dbt_score.rule import RuleViolation
 from dbt_score.scoring import Score
 
 
@@ -40,8 +40,6 @@ def model_evaluated(
         for rule, result in results.items():
             if result is None:
                 print(f"{self.indent}{self.label_ok} {rule.source()}")
-            elif isinstance(result, SkipRule):
-                continue
             elif isinstance(result, RuleViolation):
                 print(
                     f"{self.indent}{self.label_warning} "

src/dbt_score/rule.py

@@ -54,14 +54,7 @@ class RuleViolation:
     message: str | None = None
 
 
-@dataclass
-class SkipRule:
-    """This evaluation of the rule should be skipped."""
-
-    pass
-
-
-RuleEvaluationType: TypeAlias = Callable[[Model], RuleViolation | SkipRule | None]
+RuleEvaluationType: TypeAlias = Callable[[Model], RuleViolation | None]
 
 
 class Rule:
@@ -104,7 +97,7 @@ def process_config(self, rule_config: RuleConfig) -> None:
         self.model_filter_names = rule_config.model_filter_names
         self.config = config
 
-    def evaluate(self, model: Model) -> RuleViolation | SkipRule | None:
+    def evaluate(self, model: Model) -> RuleViolation | None:
         """Evaluates the rule."""
         raise NotImplementedError("Subclass must implement method `evaluate`.")
 
@@ -188,9 +181,7 @@ def decorator_rule(
             func.__doc__.split("\n")[0] if func.__doc__ else None
         )
 
-        def wrapped_func(
-            self: Rule, *args: Any, **kwargs: Any
-        ) -> RuleViolation | SkipRule | None:
+        def wrapped_func(self: Rule, *args: Any, **kwargs: Any) -> RuleViolation | None:
             """Wrap func to add `self`."""
             return func(*args, **kwargs)
 

src/dbt_score/scoring.py

@@ -9,7 +9,7 @@
 
 if typing.TYPE_CHECKING:
     from dbt_score.evaluation import ModelResultsType
-from dbt_score.rule import RuleViolation, Severity, SkipRule
+from dbt_score.rule import RuleViolation, Severity
 
 
 @dataclass
@@ -39,11 +39,7 @@ def __init__(self, config: Config) -> None:
 
     def score_model(self, model_results: ModelResultsType) -> Score:
         """Compute the score of a given model."""
-        rule_count = sum(
-            1
-            for rule, result in model_results.items()
-            if not isinstance(result, SkipRule)
-        )
+        rule_count = len(model_results)
 
         if rule_count == 0:
             # No rule? No problem
@@ -64,7 +60,6 @@ def score_model(self, model_results: ModelResultsType) -> Score:
                         if isinstance(result, RuleViolation)  # Either 0/3, 1/3 or 2/3
                         else self.score_cardinality  # 3/3
                         for rule, result in model_results.items()
-                        if not isinstance(result, SkipRule)
                     ]
                 )
                 / (self.score_cardinality * rule_count)

tests/conftest.py

@@ -4,7 +4,7 @@
 from pathlib import Path
 from typing import Any, Type
 
-from dbt_score import Model, Rule, RuleViolation, Severity, SkipRule, rule
+from dbt_score import Model, Rule, RuleViolation, Severity, rule
 from dbt_score.config import Config
 from dbt_score.model_filter import ModelFilter, model_filter
 from dbt_score.models import ManifestLoader
@@ -213,19 +213,6 @@ def rule_error(model: Model) -> RuleViolation | None:
     return rule_error
 
 
-@fixture
-def rule_skippable() -> Type[Rule]:
-    """An example rule that may skip."""
-
-    @rule
-    def rule_with_skip(model: Model) -> RuleViolation | SkipRule | None:
-        """Skips for model1, passes for model2."""
-        if model.name == "model1":
-            return SkipRule()
-
-    return rule_with_skip
-
-
 @fixture
 def rule_with_filter() -> Type[Rule]:
     """An example rule that skips through a filter."""
@@ -236,9 +223,9 @@ def skip_model1(model: Model) -> bool:
         return model.name != "model1"
 
     @rule(model_filters={skip_model1()})
-    def rule_with_filter(model: Model) -> RuleViolation | SkipRule | None:
-        """Rule that always passes when not filtered."""
-        return None
+    def rule_with_filter(model: Model) -> RuleViolation | None:
+        """Rule that always fails when not filtered."""
+        return RuleViolation(message="I always fail.")
 
     return rule_with_filter
 
@@ -258,7 +245,7 @@ class RuleWithFilter(Rule):
         description = "Filter defined by a class."
         model_filters = frozenset({SkipModel1()})
 
-        def evaluate(self, model: Model) -> None:
-            return None
+        def evaluate(self, model: Model) -> RuleViolation | None:
+            return RuleViolation(message="I always fail.")
 
     return RuleWithFilter

tests/test_evaluation.py

@@ -5,7 +5,7 @@
 from dbt_score.config import Config
 from dbt_score.evaluation import Evaluation
 from dbt_score.models import ManifestLoader
-from dbt_score.rule import RuleViolation, SkipRule
+from dbt_score.rule import RuleViolation
 from dbt_score.rule_registry import RuleRegistry
 from dbt_score.scoring import Score
 
@@ -181,33 +181,6 @@ def test_evaluation_rule_with_config(
     assert evaluation.results[model2][rule_with_config] is None
 
 
-def test_evaluation_with_skip(manifest_path, default_config, rule_skippable):
-    """Test rule set to skip."""
-    manifest_loader = ManifestLoader(manifest_path)
-    mock_formatter = Mock()
-    mock_scorer = Mock()
-
-    rule_registry = RuleRegistry(default_config)
-    rule_registry._add_rule(rule_skippable)
-
-    # Ensure we get a valid Score object from the Mock
-    mock_scorer.score_model.return_value = Score(10, "🥇")
-
-    evaluation = Evaluation(
-        rule_registry=rule_registry,
-        manifest_loader=manifest_loader,
-        formatter=mock_formatter,
-        scorer=mock_scorer,
-    )
-    evaluation.evaluate()
-
-    model1 = manifest_loader.models[0]
-    model2 = manifest_loader.models[1]
-
-    assert isinstance(evaluation.results[model1][rule_skippable], SkipRule)
-    assert evaluation.results[model2][rule_skippable] is None
-
-
 def test_evaluation_with_filter(manifest_path, default_config, rule_with_filter):
     """Test rule with filter."""
     manifest_loader = ManifestLoader(manifest_path)
@@ -231,8 +204,8 @@ def test_evaluation_with_filter(manifest_path, default_config, rule_with_filter)
     model1 = manifest_loader.models[0]
     model2 = manifest_loader.models[1]
 
-    assert isinstance(evaluation.results[model1][rule_with_filter], SkipRule)
-    assert evaluation.results[model2][rule_with_filter] is None
+    assert rule_with_filter not in evaluation.results[model1]
+    assert isinstance(evaluation.results[model2][rule_with_filter], RuleViolation)
 
 
 def test_evaluation_with_class_filter(
@@ -260,5 +233,5 @@ def test_evaluation_with_class_filter(
     model1 = manifest_loader.models[0]
     model2 = manifest_loader.models[1]
 
-    assert isinstance(evaluation.results[model1][class_rule_with_filter], SkipRule)
-    assert evaluation.results[model2][class_rule_with_filter] is None
+    assert class_rule_with_filter not in evaluation.results[model1]
+    assert isinstance(evaluation.results[model2][class_rule_with_filter], RuleViolation)

tests/test_scoring.py

@@ -1,7 +1,7 @@
 """Unit tests for the scoring module."""
 
 
-from dbt_score.rule import RuleViolation, SkipRule
+from dbt_score.rule import RuleViolation
 from dbt_score.scoring import Score, Scorer
 
 
@@ -115,27 +115,6 @@ def test_scorer_model_multiple_rules(
     )
 
 
-def test_scorer_skipping_rule(default_config, rule_skippable, rule_severity_medium):
-    """Test scorer with a model that skips."""
-    scorer = Scorer(config=default_config)
-    scorer.score_model({rule_skippable: SkipRule()})
-
-    assert scorer.score_model({rule_skippable: SkipRule()}).value == 10.0
-
-    assert (
-        round(
-            scorer.score_model(
-                {
-                    rule_skippable: SkipRule(),
-                    rule_severity_medium: RuleViolation("error"),
-                }
-            ).value,
-            2,
-        )
-        == 3.33
-    )
-
-
 def test_scorer_aggregate_empty(default_config):
     """Test scorer aggregation with no results."""
     scorer = Scorer(config=default_config)