Skip to content

Commit 336f22d

Browse files
Convert median to percentile in metrics (like in measures) (#433)
Towards #387 ### Description Depends on #432 We already convert measures with agg value MEDIAN to be percentile measures. Since we're replacing measures with simple metrics, this PR does the same for simple metrics. Notes: * Learning from several previous PRs, including especially #431 , we use some inheritance to keep all the good, non-legacy code in the transformation class for metrics while still allowing a lot of reuse for the measures-related class. * I also did a little renaming and re-documenting to the test cases I added to show make it clear what was legacy and for measures and what was not. ### Checklist - [x] I have read [the contributing guide](https://github.com/dbt-labs/dbt-semantic-interfaces/blob/main/CONTRIBUTING.md) and understand what's expected of me - [x] I have signed the [CLA](https://docs.getdbt.com/docs/contributor-license-agreements) - [x] This PR includes tests, or tests are not required/relevant for this PR - [x] I have run `changie new` to [create a changelog entry](https://github.com/dbt-labs/dbt-semantic-interfaces/blob/main/CONTRIBUTING.md#adding-a-changelog-entry)
1 parent 65b1ddb commit 336f22d

File tree

3 files changed

+374
-21
lines changed

3 files changed

+374
-21
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
kind: Under the Hood
2+
body: Add transformation for new simple metrics that matches the one for measures with MEDIAN agg types.
3+
time: 2025-09-29T14:32:50.563028-07:00
4+
custom:
5+
Author: theyostalservice
6+
Issue: "387"
Lines changed: 87 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from typing import Literal, NoReturn
2+
13
from typing_extensions import override
24

35
from dbt_semantic_interfaces.errors import ModelTransformError
@@ -12,11 +14,80 @@
1214
SemanticManifestTransformRule,
1315
)
1416
from dbt_semantic_interfaces.type_enums import AggregationType
17+
from dbt_semantic_interfaces.type_enums.metric_type import MetricType
18+
19+
20+
class ConvertMedianMetricToPercentile(ProtocolHint[SemanticManifestTransformRule[PydanticSemanticManifest]]):
21+
"""Converts any MEDIAN metrics to percentile equivalent.
1522
16-
MEDIAN_PERCENTILE = 0.5
23+
Applies to SIMPLE metrics that aggregate an expression directly via
24+
`type_params.metric_aggregation_params`.
25+
"""
1726

27+
TRANSFORMED_AGG_TYPE = AggregationType.PERCENTILE
28+
MEDIAN_PERCENTILE = 0.5
29+
30+
@override
31+
def _implements_protocol(self) -> SemanticManifestTransformRule[PydanticSemanticManifest]: # noqa: D
32+
return self
33+
34+
@staticmethod
35+
def _throw_conflicting_percentile_error(
36+
object_name: str,
37+
object_type: Literal["Metric", "Measure"],
38+
percentile: float,
39+
) -> NoReturn:
40+
raise ModelTransformError(
41+
f"{object_type} '{object_name}' uses a MEDIAN aggregation, while percentile "
42+
f"is set to '{percentile}', a conflicting value. Please remove the parameter "
43+
"or set to '0.5'."
44+
)
45+
46+
@staticmethod
47+
def _throw_conflicting_discrete_percentile_error(
48+
object_name: str,
49+
object_type: Literal["Metric", "Measure"],
50+
) -> NoReturn:
51+
raise ModelTransformError(
52+
f"{object_type} '{object_name}' uses a MEDIAN aggregation, while use_discrete_percentile "
53+
f"is set to true. Please remove the parameter or set to False."
54+
)
1855

19-
class ConvertMedianToPercentileRule(ProtocolHint[SemanticManifestTransformRule[PydanticSemanticManifest]]):
56+
@staticmethod
57+
def transform_model(semantic_manifest: PydanticSemanticManifest) -> PydanticSemanticManifest: # noqa: D
58+
for metric in semantic_manifest.metrics:
59+
if (
60+
metric.type == MetricType.SIMPLE
61+
and metric.type_params.metric_aggregation_params is not None
62+
and metric.type_params.metric_aggregation_params.agg == AggregationType.MEDIAN
63+
):
64+
# Update aggregation type first
65+
metric.type_params.metric_aggregation_params.agg = ConvertMedianMetricToPercentile.TRANSFORMED_AGG_TYPE
66+
67+
# Ensure aggregation parameters exist, then validate
68+
if metric.type_params.metric_aggregation_params.agg_params is None:
69+
metric.type_params.metric_aggregation_params.agg_params = PydanticMeasureAggregationParameters()
70+
else:
71+
agg_params = metric.type_params.metric_aggregation_params.agg_params
72+
if agg_params.percentile is not None and agg_params.percentile != MEDIAN_PERCENTILE:
73+
ConvertMedianMetricToPercentile._throw_conflicting_percentile_error(
74+
object_name=metric.name,
75+
object_type="Metric",
76+
percentile=agg_params.percentile,
77+
)
78+
if agg_params.use_discrete_percentile:
79+
ConvertMedianMetricToPercentile._throw_conflicting_discrete_percentile_error(
80+
object_name=metric.name,
81+
object_type="Metric",
82+
)
83+
84+
# Set the median percentile
85+
metric.type_params.metric_aggregation_params.agg_params.percentile = MEDIAN_PERCENTILE
86+
87+
return semantic_manifest
88+
89+
90+
class ConvertMedianToPercentileRule(ConvertMedianMetricToPercentile):
2091
"""Converts any MEDIAN measures to percentile equivalent."""
2192

2293
@override
@@ -28,22 +99,27 @@ def transform_model(semantic_manifest: PydanticSemanticManifest) -> PydanticSema
2899
for semantic_model in semantic_manifest.semantic_models:
29100
for measure in semantic_model.measures:
30101
if measure.agg == AggregationType.MEDIAN:
31-
measure.agg = AggregationType.PERCENTILE
102+
measure.agg = ConvertMedianToPercentileRule.TRANSFORMED_AGG_TYPE
32103

33104
if not measure.agg_params:
34105
measure.agg_params = PydanticMeasureAggregationParameters()
35106
else:
36107
if measure.agg_params.percentile is not None and measure.agg_params.percentile != 0.5:
37-
raise ModelTransformError(
38-
f"Measure '{measure.name}' uses a MEDIAN aggregation, while percentile is set to "
39-
f"'{measure.agg_params.percentile}', a conflicting value. Please remove the parameter "
40-
"or set to '0.5'."
108+
ConvertMedianToPercentileRule._throw_conflicting_percentile_error(
109+
object_name=measure.name,
110+
object_type="Measure",
111+
percentile=measure.agg_params.percentile,
41112
)
42113
if measure.agg_params.use_discrete_percentile:
43-
raise ModelTransformError(
44-
f"Measure '{measure.name}' uses a MEDIAN aggregation, while use_discrete_percentile "
45-
f"is set to true. Please remove the parameter or set to False."
114+
ConvertMedianToPercentileRule._throw_conflicting_discrete_percentile_error(
115+
object_name=measure.name,
116+
object_type="Measure",
46117
)
47-
measure.agg_params.percentile = MEDIAN_PERCENTILE
118+
measure.agg_params.percentile = ConvertMedianToPercentileRule.MEDIAN_PERCENTILE
48119
# let's not set use_approximate_percentile to be false due to valid performance reasons
49120
return semantic_manifest
121+
122+
123+
# Just left here for legacy reasons. Maybe we can get rid of this when we remove
124+
# measures?
125+
MEDIAN_PERCENTILE = ConvertMedianMetricToPercentile.MEDIAN_PERCENTILE

0 commit comments

Comments
 (0)