Skip to content

Commit

Permalink
Rename Metric.default_granularity field to Metric.time_granularity (
Browse files Browse the repository at this point in the history
#310)

### Description
After some discussion with product and dev experience, we've decided to
change the name of this field. It hasn't shipped to core yet so is not a
breaking change. The name change is intended to remove the word
"default", which should be implied for a YAML field. This is more
consistent with other dbt YAML objects, which have a default in YAML
that can be overridden at query time. The name `time_granularity` is
also more consistent with the field we have on time dimensions.
Everything in this PR is just a find & replace operation.

### 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)
  • Loading branch information
courtneyholcomb authored Jul 11, 2024
1 parent 5e9a3dd commit 4d5de68
Show file tree
Hide file tree
Showing 13 changed files with 87 additions and 89 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Under the Hood-20240711-105519.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Under the Hood
body: Rename Metric.default_granularity field to Metric.time_granularity.
time: 2024-07-11T10:55:19.887705-07:00
custom:
Author: courtneyholcomb
Issue: "290"
2 changes: 1 addition & 1 deletion dbt_semantic_interfaces/implementations/metric.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ def _implements_protocol(self) -> Metric: # noqa: D
metadata: Optional[PydanticMetadata]
label: Optional[str] = None
config: Optional[PydanticMetricConfig]
default_granularity: Optional[TimeGranularity] = None
time_granularity: Optional[TimeGranularity] = None

@property
def input_measures(self) -> Sequence[PydanticMetricInputMeasure]:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,20 @@
"config": {
"$ref": "#/definitions/metric_config_schema"
},
"default_granularity": {
"description": {
"type": "string"
},
"filter": {
"$ref": "#/definitions/filter_schema"
},
"label": {
"type": "string"
},
"name": {
"pattern": "(?!.*__).*^[a-z][a-z0-9_]*[a-z0-9]$",
"type": "string"
},
"time_granularity": {
"enum": [
"NANOSECOND",
"MICROSECOND",
Expand All @@ -484,19 +497,6 @@
"year"
]
},
"description": {
"type": "string"
},
"filter": {
"$ref": "#/definitions/filter_schema"
},
"label": {
"type": "string"
},
"name": {
"pattern": "(?!.*__).*^[a-z][a-z0-9_]*[a-z0-9]$",
"type": "string"
},
"type": {
"enum": [
"SIMPLE",
Expand Down
2 changes: 1 addition & 1 deletion dbt_semantic_interfaces/parsing/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@
"description": {"type": "string"},
"label": {"type": "string"},
"config": {"$ref": "metric_config_schema"},
"default_granularity": {"enum": time_granularity_values},
"time_granularity": {"enum": time_granularity_values},
},
"additionalProperties": False,
"required": ["name", "type", "type_params"],
Expand Down
2 changes: 1 addition & 1 deletion dbt_semantic_interfaces/protocols/metric.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ def label(self) -> Optional[str]:

@property
@abstractmethod
def default_granularity(self) -> Optional[TimeGranularity]:
def time_granularity(self) -> Optional[TimeGranularity]:
"""Default grain used for the metric.
This will be used in a couple of circumstances:
Expand Down
4 changes: 2 additions & 2 deletions dbt_semantic_interfaces/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def metric_with_guaranteed_meta(
type_params: PydanticMetricTypeParams,
metadata: PydanticMetadata = default_meta(),
description: str = "adhoc metric",
default_granularity: Optional[TimeGranularity] = None,
time_granularity: Optional[TimeGranularity] = None,
) -> PydanticMetric:
"""Creates a metric with the given input.
Expand All @@ -136,7 +136,7 @@ def metric_with_guaranteed_meta(
type_params=type_params,
filter=None,
metadata=metadata,
default_granularity=default_granularity,
time_granularity=time_granularity,
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,21 @@
logger = logging.getLogger(__name__)


class SetDefaultGranularityRule(ProtocolHint[SemanticManifestTransformRule[PydanticSemanticManifest]]):
"""If default_granularity is not set for a metric, set it to DAY if available, else the smallest available grain."""
class SetMetricTimeGranularityRule(ProtocolHint[SemanticManifestTransformRule[PydanticSemanticManifest]]):
"""If time_granularity is not set for a metric, set it to DAY if available, else the smallest available grain."""

@override
def _implements_protocol(self) -> SemanticManifestTransformRule[PydanticSemanticManifest]: # noqa: D
return self

@staticmethod
def transform_model(semantic_manifest: PydanticSemanticManifest) -> PydanticSemanticManifest:
"""For each metric, set default_granularity to DAY or smallest granularity supported by all agg_time_dims."""
"""For each metric, set time_granularity to DAY or smallest granularity supported by all agg_time_dims."""
for metric in semantic_manifest.metrics:
if metric.default_granularity:
if metric.time_granularity:
continue

default_granularity = TimeGranularity.DAY
time_granularity = TimeGranularity.DAY
seen_agg_time_dimensions: Set[Tuple[SemanticModelReference, TimeDimensionReference]] = set()

metric_index: Dict[MetricReference, Metric] = {
Expand Down Expand Up @@ -66,10 +66,10 @@ def transform_model(semantic_manifest: PydanticSemanticManifest) -> PydanticSema
dimension = semantic_model.get_dimension(DimensionReference(agg_time_dimension_ref.element_name))
if (
dimension.type_params
and dimension.type_params.time_granularity.to_int() > default_granularity.to_int()
and dimension.type_params.time_granularity.to_int() > time_granularity.to_int()
):
default_granularity = dimension.type_params.time_granularity
time_granularity = dimension.type_params.time_granularity

metric.default_granularity = default_granularity
metric.time_granularity = time_granularity

return semantic_manifest
6 changes: 3 additions & 3 deletions dbt_semantic_interfaces/transformations/pydantic_rule_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
from dbt_semantic_interfaces.transformations.cumulative_type_params import (
SetCumulativeTypeParamsRule,
)
from dbt_semantic_interfaces.transformations.default_granularity import (
SetDefaultGranularityRule,
from dbt_semantic_interfaces.transformations.metric_time_granularity import (
SetMetricTimeGranularityRule,
)
from dbt_semantic_interfaces.transformations.names import LowerCaseNamesRule
from dbt_semantic_interfaces.transformations.proxy_measure import CreateProxyMeasureRule
Expand Down Expand Up @@ -57,7 +57,7 @@ def secondary_rules(self) -> Sequence[SemanticManifestTransformRule[PydanticSema
ConvertMedianToPercentileRule(),
AddInputMetricMeasuresRule(),
SetCumulativeTypeParamsRule(),
SetDefaultGranularityRule(),
SetMetricTimeGranularityRule(),
)

@property
Expand Down
24 changes: 12 additions & 12 deletions dbt_semantic_interfaces/validations/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,8 +574,8 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati
return issues


class DefaultGranularityRule(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]):
"""Checks that default_granularity set for metric is queryable for that metric."""
class MetricTimeGranularityRule(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]):
"""Checks that time_granularity set for metric is queryable for that metric."""

@staticmethod
def _min_queryable_granularity_for_metric(
Expand Down Expand Up @@ -606,7 +606,7 @@ def _min_queryable_granularity_for_metric(

@staticmethod
@validate_safely(
whats_being_done="running model validation ensuring a metric's default_granularity is valid for the metric"
whats_being_done="running model validation ensuring a metric's time_granularity is valid for the metric"
)
def _validate_metric(
metric: Metric,
Expand All @@ -619,16 +619,16 @@ def _validate_metric(
metric=MetricModelReference(metric_name=metric.name),
)

if metric.default_granularity:
min_queryable_granularity = DefaultGranularityRule._min_queryable_granularity_for_metric(
if metric.time_granularity:
min_queryable_granularity = MetricTimeGranularityRule._min_queryable_granularity_for_metric(
metric=metric, metric_index=metric_index, measure_to_agg_time_dimension=measure_to_agg_time_dimension
)
if not min_queryable_granularity:
issues.append(
ValidationError(
context=context,
message=(
f"Unable to validate `default_granularity` for metric '{metric.name}' due to "
f"Unable to validate `time_granularity` for metric '{metric.name}' due to "
"misconfiguration with measures or related agg_time_dimensions."
),
)
Expand All @@ -639,25 +639,25 @@ def _validate_metric(
for granularity in TimeGranularity
if granularity.to_int() >= min_queryable_granularity.to_int()
]
if metric.default_granularity.name not in valid_granularities:
if metric.time_granularity.name not in valid_granularities:
issues.append(
ValidationError(
context=context,
message=(
f"`default_granularity` for metric '{metric.name}' must be >= "
f"`time_granularity` for metric '{metric.name}' must be >= "
f"{min_queryable_granularity.name}. Valid options are those that are >= the largest "
f"granularity defined for the metric's measures' agg_time_dimensions. Got: "
f"{metric.default_granularity.name}. Valid options: {valid_granularities}"
f"{metric.time_granularity.name}. Valid options: {valid_granularities}"
),
)
)

return issues

@staticmethod
@validate_safely(whats_being_done="running manifest validation ensuring metric default_granularitys are valid")
@validate_safely(whats_being_done="running manifest validation ensuring metric time_granularitys are valid")
def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]:
"""Validate that the default_granularity for each metric is queryable for that metric.
"""Validate that the time_granularity for each metric is queryable for that metric.
TODO: figure out a more efficient way to reference other aspects of the model. This validation essentially
requires parsing the entire model, which could be slow and likely is repeated work. The blocker is that the
Expand All @@ -682,7 +682,7 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati

metric_index = {MetricReference(metric.name): metric for metric in semantic_manifest.metrics}
for metric in semantic_manifest.metrics or []:
issues += DefaultGranularityRule._validate_metric(
issues += MetricTimeGranularityRule._validate_metric(
metric=metric,
metric_index=metric_index,
measure_to_agg_time_dimension=measure_to_agg_time_dimension,
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "dbt-semantic-interfaces"
version = "0.6.5"
version = "0.6.6"
description = 'The shared semantic layer definitions that dbt-core and MetricFlow use'
readme = "README.md"
requires-python = ">=3.8"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ metric:
name: "trailing_2_months_revenue"
description: "trailing_2_months_revenue"
type: cumulative
default_granularity: month
time_granularity: month
type_params:
measure:
name: txn_revenue
Expand Down
28 changes: 14 additions & 14 deletions tests/transformations/test_configurable_transform_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
from dbt_semantic_interfaces.implementations.semantic_manifest import (
PydanticSemanticManifest,
)
from dbt_semantic_interfaces.transformations.default_granularity import (
SetDefaultGranularityRule,
from dbt_semantic_interfaces.transformations.metric_time_granularity import (
SetMetricTimeGranularityRule,
)
from dbt_semantic_interfaces.transformations.semantic_manifest_transformer import (
PydanticSemanticManifestTransformer,
Expand Down Expand Up @@ -40,30 +40,30 @@ def test_can_configure_model_transform_rules( # noqa: D
assert all(len(x.name) == 3 for x in transformed_model.semantic_models)


def test_set_default_granularity_rule( # noqa: D
def test_set_time_granularity_rule( # noqa: D
simple_semantic_manifest__with_primary_transforms: PydanticSemanticManifest,
) -> None:
pre_model = simple_semantic_manifest__with_primary_transforms

metric_exists_without_default_granularity = False
metric_exists_without_time_granularity = False
configured_default_granularities: Dict[str, TimeGranularity] = {}
for metric in pre_model.metrics:
if metric.default_granularity:
configured_default_granularities[metric.name] = metric.default_granularity
metric_exists_without_default_granularity = True
if metric.time_granularity:
configured_default_granularities[metric.name] = metric.time_granularity
metric_exists_without_time_granularity = True

assert (
pre_model.metrics and metric_exists_without_default_granularity
), "If there are no metrics without a configured default_granularity, this tests nothing."
pre_model.metrics and metric_exists_without_time_granularity
), "If there are no metrics without a configured time_granularity, this tests nothing."

rules = [SetDefaultGranularityRule()]
rules = [SetMetricTimeGranularityRule()]
transformed_model = PydanticSemanticManifestTransformer.transform(pre_model, ordered_rule_sequences=(rules,))

for metric in transformed_model.metrics:
assert metric.default_granularity, f"No default_granularity set in transformation for metric '{metric.name}'"
assert metric.time_granularity, f"No time_granularity set in transformation for metric '{metric.name}'"
if metric.name in configured_default_granularities:
assert (
metric.default_granularity == configured_default_granularities[metric.name]
), f"Default granularity was unexpected changed during transformation for metric '{metric.name}"
metric.time_granularity == configured_default_granularities[metric.name]
), f"Time granularity was unexpected changed during transformation for metric '{metric.name}"
if metric.name == "monthly_times_yearly_bookings":
assert metric.default_granularity == TimeGranularity.YEAR
assert metric.time_granularity == TimeGranularity.YEAR
Loading

0 comments on commit 4d5de68

Please sign in to comment.