From 3ea5fc207ce19ac779b97bdd001c93497fa3e24f Mon Sep 17 00:00:00 2001 From: Will Deng Date: Tue, 14 Nov 2023 13:14:26 -0500 Subject: [PATCH 1/9] added protocols for ConversionMetric properties --- dbt_semantic_interfaces/protocols/metric.py | 63 ++++++++++++++++++- .../type_enums/__init__.py | 3 + .../type_enums/conversion_calculation_type.py | 8 +++ .../type_enums/metric_type.py | 1 + 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 dbt_semantic_interfaces/type_enums/conversion_calculation_type.py diff --git a/dbt_semantic_interfaces/protocols/metric.py b/dbt_semantic_interfaces/protocols/metric.py index 31f21682..95d1dfe3 100644 --- a/dbt_semantic_interfaces/protocols/metric.py +++ b/dbt_semantic_interfaces/protocols/metric.py @@ -6,7 +6,11 @@ from dbt_semantic_interfaces.protocols.metadata import Metadata from dbt_semantic_interfaces.protocols.where_filter import WhereFilterIntersection from dbt_semantic_interfaces.references import MeasureReference, MetricReference -from dbt_semantic_interfaces.type_enums import MetricType, TimeGranularity +from dbt_semantic_interfaces.type_enums import ( + ConversionCalculationType, + MetricType, + TimeGranularity, +) class MetricInputMeasure(Protocol): @@ -113,6 +117,52 @@ def post_aggregation_reference(self) -> MetricReference: pass +class ConversionTypeParams(Protocol): + """Type params to provide context for conversion metrics properties.""" + + @property + @abstractmethod + def base_measure(self) -> MetricInputMeasure: + """Measure used to calculate the base event.""" + pass + + @property + @abstractmethod + def conversion_measure(self) -> MetricInputMeasure: + """Measure used to calculate the conversion event.""" + pass + + @property + @abstractmethod + def entity(self) -> str: + """Specified join entity.""" + pass + + @property + @abstractmethod + def calculation(self) -> ConversionCalculationType: + """Type of conversion metric calculation.""" + pass + + @property + @abstractmethod + def window(self) -> Optional[MetricTimeWindow]: + """Maximum time range for finding successing conversion events.""" + pass + + @property + @abstractmethod + def base_measure_reference(self) -> MeasureReference: + """Return the measure reference associated with the base measure.""" + pass + + @property + @abstractmethod + def conversion_measure_reference(self) -> MeasureReference: + """Return the measure reference associated with the conversion measure.""" + pass + + class MetricTypeParams(Protocol): """Type params add additional context to certain metric types (the context depends on the metric type).""" @@ -157,6 +207,11 @@ def grain_to_date(self) -> Optional[TimeGranularity]: # noqa: D def metrics(self) -> Optional[Sequence[MetricInput]]: # noqa: D pass + @property + @abstractmethod + def conversion_type_params(self) -> Optional[ConversionTypeParams]: # noqa: D + pass + class Metric(Protocol): """Describes a metric.""" @@ -215,3 +270,9 @@ def metadata(self) -> Optional[Metadata]: # noqa: D def label(self) -> Optional[str]: """Returns a string representing a human readable label for the metric.""" pass + + @property + @abstractmethod + def conversion_params(self) -> ConversionTypeParams: + """Accessor for conversion type params, enforces that it's set.""" + pass diff --git a/dbt_semantic_interfaces/type_enums/__init__.py b/dbt_semantic_interfaces/type_enums/__init__.py index 68b50b85..0389d2c4 100644 --- a/dbt_semantic_interfaces/type_enums/__init__.py +++ b/dbt_semantic_interfaces/type_enums/__init__.py @@ -1,6 +1,9 @@ from dbt_semantic_interfaces.type_enums.aggregation_type import ( # noqa:F401 AggregationType, ) +from dbt_semantic_interfaces.type_enums.conversion_calculation_type import ( # noqa:F401 + ConversionCalculationType, +) from dbt_semantic_interfaces.type_enums.dimension_type import DimensionType # noqa:F401 from dbt_semantic_interfaces.type_enums.entity_type import EntityType # noqa:F401 from dbt_semantic_interfaces.type_enums.metric_type import MetricType # noqa:F401 diff --git a/dbt_semantic_interfaces/type_enums/conversion_calculation_type.py b/dbt_semantic_interfaces/type_enums/conversion_calculation_type.py new file mode 100644 index 00000000..c112128c --- /dev/null +++ b/dbt_semantic_interfaces/type_enums/conversion_calculation_type.py @@ -0,0 +1,8 @@ +from dbt_semantic_interfaces.enum_extension import ExtendedEnum + + +class ConversionCalculationType(ExtendedEnum): + """Types of calculations for a conversion metric.""" + + CONVERSIONS = "conversions" + CONVERSION_RATE = "conversion_rate" diff --git a/dbt_semantic_interfaces/type_enums/metric_type.py b/dbt_semantic_interfaces/type_enums/metric_type.py index 36286014..2b107e71 100644 --- a/dbt_semantic_interfaces/type_enums/metric_type.py +++ b/dbt_semantic_interfaces/type_enums/metric_type.py @@ -8,3 +8,4 @@ class MetricType(ExtendedEnum): RATIO = "ratio" CUMULATIVE = "cumulative" DERIVED = "derived" + CONVERSION = "conversion" From 301284f68e646cc8105d07dabc5b893802878bdd Mon Sep 17 00:00:00 2001 From: Will Deng Date: Tue, 14 Nov 2023 13:14:43 -0500 Subject: [PATCH 2/9] added implementation for ConversionMetric properties --- .../implementations/metric.py | 35 +++++++++++++++++-- dbt_semantic_interfaces/protocols/metric.py | 11 +++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/dbt_semantic_interfaces/implementations/metric.py b/dbt_semantic_interfaces/implementations/metric.py index 656e09c5..3749e1a9 100644 --- a/dbt_semantic_interfaces/implementations/metric.py +++ b/dbt_semantic_interfaces/implementations/metric.py @@ -17,7 +17,11 @@ ) from dbt_semantic_interfaces.implementations.metadata import PydanticMetadata from dbt_semantic_interfaces.references import MeasureReference, MetricReference -from dbt_semantic_interfaces.type_enums import MetricType, TimeGranularity +from dbt_semantic_interfaces.type_enums import ( + ConversionCalculationType, + MetricType, + TimeGranularity, +) class PydanticMetricInputMeasure(PydanticCustomInputParser, HashableBaseModel): @@ -134,6 +138,26 @@ def post_aggregation_reference(self) -> MetricReference: return MetricReference(element_name=self.alias or self.name) +class PydanticConversionTypeParams(HashableBaseModel): + """Type params to provide context for conversion metrics properties.""" + + base_measure: PydanticMetricInputMeasure + conversion_measure: PydanticMetricInputMeasure + entity: str + calculation: ConversionCalculationType = ConversionCalculationType.CONVERSION_RATE + window: Optional[PydanticMetricTimeWindow] + + @property + def base_measure_reference(self) -> MeasureReference: + """Return the measure reference associated with the base measure.""" + return self.base_measure.measure_reference + + @property + def conversion_measure_reference(self) -> MeasureReference: + """Return the measure reference associated with the conversion measure.""" + return self.conversion_measure.measure_reference + + class PydanticMetricTypeParams(HashableBaseModel): """Type params add additional context to certain metric types (the context depends on the metric type).""" @@ -144,9 +168,16 @@ class PydanticMetricTypeParams(HashableBaseModel): window: Optional[PydanticMetricTimeWindow] grain_to_date: Optional[TimeGranularity] metrics: Optional[List[PydanticMetricInput]] + conversion_type_params: Optional[PydanticConversionTypeParams] input_measures: List[PydanticMetricInputMeasure] = Field(default_factory=list) + @property + def conversion_type_params_or_error(self) -> PydanticConversionTypeParams: # noqa: D + if self.conversion_type_params is None: + raise ValueError("Expected conversion_type_params to be not None.") + return self.conversion_type_params + class PydanticMetric(HashableBaseModel, ModelWithMetadataParsing): """Describes a metric.""" @@ -172,7 +203,7 @@ def measure_references(self) -> List[MeasureReference]: @property def input_metrics(self) -> Sequence[PydanticMetricInput]: """Return the associated input metrics for this metric.""" - if self.type is MetricType.SIMPLE or self.type is MetricType.CUMULATIVE: + if self.type is MetricType.SIMPLE or self.type is MetricType.CUMULATIVE or self.type is MetricType.CONVERSION: return () elif self.type is MetricType.DERIVED: assert self.type_params.metrics is not None, f"{MetricType.DERIVED} should have type_params.metrics set" diff --git a/dbt_semantic_interfaces/protocols/metric.py b/dbt_semantic_interfaces/protocols/metric.py index 95d1dfe3..2be77e6c 100644 --- a/dbt_semantic_interfaces/protocols/metric.py +++ b/dbt_semantic_interfaces/protocols/metric.py @@ -212,6 +212,11 @@ def metrics(self) -> Optional[Sequence[MetricInput]]: # noqa: D def conversion_type_params(self) -> Optional[ConversionTypeParams]: # noqa: D pass + @property + @abstractmethod + def conversion_type_params_or_error(self) -> ConversionTypeParams: # noqa: D + pass + class Metric(Protocol): """Describes a metric.""" @@ -270,9 +275,3 @@ def metadata(self) -> Optional[Metadata]: # noqa: D def label(self) -> Optional[str]: """Returns a string representing a human readable label for the metric.""" pass - - @property - @abstractmethod - def conversion_params(self) -> ConversionTypeParams: - """Accessor for conversion type params, enforces that it's set.""" - pass From 130974bacebb2fb5ea17c72e9536fe56db80731e Mon Sep 17 00:00:00 2001 From: Will Deng Date: Thu, 16 Nov 2023 13:17:01 -0500 Subject: [PATCH 3/9] updated transformation step --- .../transformations/add_input_metric_measures.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dbt_semantic_interfaces/transformations/add_input_metric_measures.py b/dbt_semantic_interfaces/transformations/add_input_metric_measures.py index c29bce8f..844098db 100644 --- a/dbt_semantic_interfaces/transformations/add_input_metric_measures.py +++ b/dbt_semantic_interfaces/transformations/add_input_metric_measures.py @@ -42,6 +42,9 @@ def _get_measures_for_metric( measures.update( AddInputMetricMeasuresRule._get_measures_for_metric(semantic_manifest, input_metric.name) ) + elif matched_metric.type is MetricType.CONVERSION: + measures.add(matched_metric.type_params.conversion_type_params_or_error.base_measure) + measures.add(matched_metric.type_params.conversion_type_params_or_error.conversion_measure) else: assert_values_exhausted(matched_metric.type) else: From f03629bfffccb28a5267d11eaeb4618dce3fddeb Mon Sep 17 00:00:00 2001 From: Will Deng Date: Sun, 26 Nov 2023 15:05:29 -0500 Subject: [PATCH 4/9] added spec changes for constant properties --- .../implementations/metric.py | 8 +++++++ dbt_semantic_interfaces/protocols/metric.py | 23 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/dbt_semantic_interfaces/implementations/metric.py b/dbt_semantic_interfaces/implementations/metric.py index 3749e1a9..a3c77277 100644 --- a/dbt_semantic_interfaces/implementations/metric.py +++ b/dbt_semantic_interfaces/implementations/metric.py @@ -118,6 +118,13 @@ def parse(window: str) -> PydanticMetricTimeWindow: ) +class PydanticConstantPropertyInput(HashableBaseModel): + """Input of a constant property used in conversion metrics.""" + + base_property: str + conversion_property: str + + class PydanticMetricInput(HashableBaseModel): """Provides a pointer to a metric along with the additional properties used on that metric.""" @@ -146,6 +153,7 @@ class PydanticConversionTypeParams(HashableBaseModel): entity: str calculation: ConversionCalculationType = ConversionCalculationType.CONVERSION_RATE window: Optional[PydanticMetricTimeWindow] + constant_properties: Optional[List[PydanticConstantPropertyInput]] @property def base_measure_reference(self) -> MeasureReference: diff --git a/dbt_semantic_interfaces/protocols/metric.py b/dbt_semantic_interfaces/protocols/metric.py index 2be77e6c..0d907b9a 100644 --- a/dbt_semantic_interfaces/protocols/metric.py +++ b/dbt_semantic_interfaces/protocols/metric.py @@ -117,6 +117,23 @@ def post_aggregation_reference(self) -> MetricReference: pass +class ConstantPropertyInput(Protocol): + """Provides the constant property values for conversion metrics. + + The specified property will be a reference of a dimension/entity. + """ + + @property + @abstractmethod + def base_property(self) -> str: # noqa: D + pass + + @property + @abstractmethod + def conversion_property(self) -> str: # noqa: D + pass + + class ConversionTypeParams(Protocol): """Type params to provide context for conversion metrics properties.""" @@ -162,6 +179,12 @@ def conversion_measure_reference(self) -> MeasureReference: """Return the measure reference associated with the conversion measure.""" pass + @property + @abstractmethod + def constant_properties(self) -> Optional[Sequence[ConstantPropertyInput]]: + """Return the list of defined constant properties.""" + pass + class MetricTypeParams(Protocol): """Type params add additional context to certain metric types (the context depends on the metric type).""" From b93ac79d9a9a7d661ce45e7ac499bff28c32f553 Mon Sep 17 00:00:00 2001 From: Will Deng Date: Thu, 16 Nov 2023 13:16:47 -0500 Subject: [PATCH 5/9] updated parser --- .../default_explicit_schema.json | 62 ++++++++++++++++++- dbt_semantic_interfaces/parsing/schemas.py | 34 +++++++++- 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/dbt_semantic_interfaces/parsing/generated_json_schemas/default_explicit_schema.json b/dbt_semantic_interfaces/parsing/generated_json_schemas/default_explicit_schema.json index 7384be5f..b023f370 100644 --- a/dbt_semantic_interfaces/parsing/generated_json_schemas/default_explicit_schema.json +++ b/dbt_semantic_interfaces/parsing/generated_json_schemas/default_explicit_schema.json @@ -17,6 +17,61 @@ }, "type": "object" }, + "constant_property_input_schema": { + "$id": "constant_property_input_schema", + "additionalProperties": false, + "properties": { + "base_property": { + "type": "string" + }, + "conversion_property": { + "type": "string" + } + }, + "required": [ + "base_property", + "conversion_property" + ], + "type": "object" + }, + "conversion_type_params_schema": { + "$id": "conversion_type_params_schema", + "additionalProperties": false, + "properties": { + "base_measure": { + "$ref": "#/definitions/metric_input_measure_schema" + }, + "calculation": { + "enum": [ + "CONVERSIONS", + "CONVERSION_RATE", + "conversions", + "conversion_rate" + ] + }, + "constant_properties": { + "items": { + "$ref": "#/definitions/constant_property_input_schema" + }, + "type": "array" + }, + "conversion_measure": { + "$ref": "#/definitions/metric_input_measure_schema" + }, + "entity": { + "type": "string" + }, + "window": { + "type": "string" + } + }, + "required": [ + "base_measure", + "conversion_measure", + "entity" + ], + "type": "object" + }, "dimension_schema": { "$id": "dimension_schema", "additionalProperties": false, @@ -347,10 +402,12 @@ "RATIO", "CUMULATIVE", "DERIVED", + "CONVERSION", "simple", "ratio", "cumulative", - "derived" + "derived", + "conversion" ] }, "type_params": { @@ -368,6 +425,9 @@ "$id": "metric_type_params", "additionalProperties": false, "properties": { + "conversion_type_params": { + "$ref": "#/definitions/conversion_type_params_schema" + }, "denominator": { "$ref": "#/definitions/metric_input_measure_schema" }, diff --git a/dbt_semantic_interfaces/parsing/schemas.py b/dbt_semantic_interfaces/parsing/schemas.py index aafdc4b4..5636a7db 100644 --- a/dbt_semantic_interfaces/parsing/schemas.py +++ b/dbt_semantic_interfaces/parsing/schemas.py @@ -9,9 +9,12 @@ # Enums -metric_types_enum_values = ["SIMPLE", "RATIO", "CUMULATIVE", "DERIVED"] +metric_types_enum_values = ["SIMPLE", "RATIO", "CUMULATIVE", "DERIVED", "CONVERSION"] metric_types_enum_values += [x.lower() for x in metric_types_enum_values] +calculation_types_enum_values = ["CONVERSIONS", "CONVERSION_RATE"] +calculation_types_enum_values += [x.lower() for x in calculation_types_enum_values] + entity_type_enum_values = ["PRIMARY", "UNIQUE", "FOREIGN", "NATURAL"] entity_type_enum_values += [x.lower() for x in entity_type_enum_values] @@ -85,6 +88,32 @@ "additionalProperties": False, } +conversion_type_params_schema = { + "$id": "conversion_type_params_schema", + "type": "object", + "properties": { + "base_measure": {"$ref": "metric_input_measure_schema"}, + "conversion_measure": {"$ref": "metric_input_measure_schema"}, + "calculation": {"enum": calculation_types_enum_values}, + "entity": {"type": "string"}, + "window": {"type": "string"}, + "constant_properties": {"type": "array", "items": {"$ref": "constant_property_input_schema"}}, + }, + "additionalProperties": False, + "required": ["base_measure", "conversion_measure", "entity"], +} + +constant_property_input_schema = { + "$id": "constant_property_input_schema", + "type": "object", + "properties": { + "base_property": {"type": "string"}, + "conversion_property": {"type": "string"}, + }, + "additionalProperties": False, + "required": ["base_property", "conversion_property"], +} + metric_type_params_schema = { "$id": "metric_type_params", "type": "object", @@ -99,6 +128,7 @@ "type": "array", "items": {"$ref": "metric_input_schema"}, }, + "conversion_type_params": {"$ref": "conversion_type_params_schema"}, }, "additionalProperties": False, } @@ -382,6 +412,8 @@ filter_schema["$id"]: filter_schema, metric_input_measure_schema["$id"]: metric_input_measure_schema, metric_type_params_schema["$id"]: metric_type_params_schema, + conversion_type_params_schema["$id"]: conversion_type_params_schema, + constant_property_input_schema["$id"]: constant_property_input_schema, entity_schema["$id"]: entity_schema, measure_schema["$id"]: measure_schema, dimension_schema["$id"]: dimension_schema, From 37f64e7734aac909c2cceb6d75e86c4652423ded Mon Sep 17 00:00:00 2001 From: Will Deng Date: Mon, 27 Nov 2023 12:46:06 -0500 Subject: [PATCH 6/9] changelog --- .changes/unreleased/Features-20231127-124601.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Features-20231127-124601.yaml diff --git a/.changes/unreleased/Features-20231127-124601.yaml b/.changes/unreleased/Features-20231127-124601.yaml new file mode 100644 index 00000000..69ce1b99 --- /dev/null +++ b/.changes/unreleased/Features-20231127-124601.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Added spec changes for conversion metrics. +time: 2023-11-27T12:46:01.036548-05:00 +custom: + Author: WilliamDee + Issue: "210" From e714d89369e03c09f38c15aa8a922b3cf1005a7f Mon Sep 17 00:00:00 2001 From: Will Deng Date: Mon, 27 Nov 2023 12:58:54 -0500 Subject: [PATCH 7/9] added tests --- tests/parsing/test_metric_parsing.py | 84 +++++++++++++++++++++- tests/test_implements_satisfy_protocols.py | 12 ++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/tests/parsing/test_metric_parsing.py b/tests/parsing/test_metric_parsing.py index dba46209..7e71cb9f 100644 --- a/tests/parsing/test_metric_parsing.py +++ b/tests/parsing/test_metric_parsing.py @@ -13,7 +13,11 @@ parse_yaml_files_to_semantic_manifest, ) from dbt_semantic_interfaces.parsing.objects import YamlConfigFile -from dbt_semantic_interfaces.type_enums import MetricType, TimeGranularity +from dbt_semantic_interfaces.type_enums import ( + ConversionCalculationType, + MetricType, + TimeGranularity, +) from dbt_semantic_interfaces.validations.validator_helpers import ( SemanticManifestValidationException, ) @@ -410,6 +414,84 @@ def test_derived_metric_input_parsing() -> None: ) +def test_conversion_metric_parsing() -> None: + """Test for parsing a conversion metric.""" + yaml_contents = textwrap.dedent( + """\ + metric: + name: conversion_metric + type: conversion + type_params: + conversion_type_params: + base_measure: opportunity + conversion_measure: conversions + window: 7 days + entity: user + """ + ) + file = YamlConfigFile(filepath="inline_for_test", contents=yaml_contents) + + build_result = parse_yaml_files_to_semantic_manifest(files=[file, EXAMPLE_PROJECT_CONFIGURATION_YAML_CONFIG_FILE]) + + assert len(build_result.semantic_manifest.metrics) == 1 + metric = build_result.semantic_manifest.metrics[0] + assert metric.name == "conversion_metric" + assert metric.type is MetricType.CONVERSION + assert metric.type_params + assert metric.type_params.conversion_type_params + assert metric.type_params.conversion_type_params.base_measure == PydanticMetricInputMeasure(name="opportunity") + assert metric.type_params.conversion_type_params.conversion_measure == PydanticMetricInputMeasure( + name="conversions" + ) + assert metric.type_params.conversion_type_params.window == PydanticMetricTimeWindow( + count=7, granularity=TimeGranularity.DAY + ) + assert metric.type_params.conversion_type_params.entity == "user" + assert metric.type_params.conversion_type_params.calculation == ConversionCalculationType.CONVERSION_RATE + + +def test_conversion_metric_parsing_with_constant_properties() -> None: + """Test for parsing a conversion metric with specified constant properties.""" + yaml_contents = textwrap.dedent( + """\ + metric: + name: conversion_metric + type: conversion + type_params: + conversion_type_params: + base_measure: opportunity + conversion_measure: conversions + entity: user + calculation: conversions + constant_properties: + - base_property: base_session_id + conversion_property: conversion_session_id + """ + ) + file = YamlConfigFile(filepath="inline_for_test", contents=yaml_contents) + + build_result = parse_yaml_files_to_semantic_manifest(files=[file, EXAMPLE_PROJECT_CONFIGURATION_YAML_CONFIG_FILE]) + + assert len(build_result.semantic_manifest.metrics) == 1 + metric = build_result.semantic_manifest.metrics[0] + assert metric.name == "conversion_metric" + assert metric.type is MetricType.CONVERSION + assert metric.type_params + assert metric.type_params.conversion_type_params + assert metric.type_params.conversion_type_params.base_measure == PydanticMetricInputMeasure(name="opportunity") + assert metric.type_params.conversion_type_params.conversion_measure == PydanticMetricInputMeasure( + name="conversions" + ) + assert metric.type_params.conversion_type_params.window is None + assert metric.type_params.conversion_type_params.entity == "user" + assert metric.type_params.conversion_type_params.calculation == ConversionCalculationType.CONVERSIONS + assert metric.type_params.conversion_type_params.constant_properties + assert metric.type_params.conversion_type_params.constant_properties[0].base_property == "base_session_id" + assert ( + metric.type_params.conversion_type_params.constant_properties[0].conversion_property == "conversion_session_id" + ) + + def test_invalid_metric_type_parsing_error() -> None: """Test for error detection when parsing a metric specification with an invalid MetricType input value.""" yaml_contents = textwrap.dedent( diff --git a/tests/test_implements_satisfy_protocols.py b/tests/test_implements_satisfy_protocols.py index c6d0fcd5..cf0d7f69 100644 --- a/tests/test_implements_satisfy_protocols.py +++ b/tests/test_implements_satisfy_protocols.py @@ -200,6 +200,18 @@ def test_metric_protocol_derived(metric: PydanticMetric) -> None: # noqa: D assert isinstance(metric, RuntimeCheckableMetric) +@given( + builds( + PydanticMetric, + type=just(MetricType.CONVERSION), + type_params=builds(PydanticMetricTypeParams, metrics=lists(builds(PydanticMetricInput))), + expr=builds(str), + ) +) +def test_metric_protocol_conversion(metric: PydanticMetric) -> None: # noqa: D + assert isinstance(metric, RuntimeCheckableMetric) + + @runtime_checkable class RuntimeCheckableEntity(EntityProtocol, Protocol): """We don't want runtime_checkable versions of protocols in the package, but we want them for tests.""" From ef61717718546bdacf98b39b2db0a18915bb7ee1 Mon Sep 17 00:00:00 2001 From: Will Deng Date: Mon, 27 Nov 2023 13:07:18 -0500 Subject: [PATCH 8/9] added new protocols to __init__ --- dbt_semantic_interfaces/protocols/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dbt_semantic_interfaces/protocols/__init__.py b/dbt_semantic_interfaces/protocols/__init__.py index 239aa83b..290c6374 100644 --- a/dbt_semantic_interfaces/protocols/__init__.py +++ b/dbt_semantic_interfaces/protocols/__init__.py @@ -11,6 +11,8 @@ ) from dbt_semantic_interfaces.protocols.metadata import FileSlice, Metadata # noqa:F401 from dbt_semantic_interfaces.protocols.metric import ( # noqa:F401 + ConstantPropertyInput, + ConversionTypeParams, Metric, MetricInput, MetricInputMeasure, From 999d667e089a1ef7d0315a82485acf8d3aba368b Mon Sep 17 00:00:00 2001 From: Will Deng Date: Wed, 29 Nov 2023 00:54:18 -0500 Subject: [PATCH 9/9] addressed reviews --- .../implementations/metric.py | 16 ----------- dbt_semantic_interfaces/protocols/metric.py | 27 +++++-------------- .../add_input_metric_measures.py | 6 +++-- tests/test_implements_satisfy_protocols.py | 11 +++++++- 4 files changed, 21 insertions(+), 39 deletions(-) diff --git a/dbt_semantic_interfaces/implementations/metric.py b/dbt_semantic_interfaces/implementations/metric.py index a3c77277..366648a4 100644 --- a/dbt_semantic_interfaces/implementations/metric.py +++ b/dbt_semantic_interfaces/implementations/metric.py @@ -155,16 +155,6 @@ class PydanticConversionTypeParams(HashableBaseModel): window: Optional[PydanticMetricTimeWindow] constant_properties: Optional[List[PydanticConstantPropertyInput]] - @property - def base_measure_reference(self) -> MeasureReference: - """Return the measure reference associated with the base measure.""" - return self.base_measure.measure_reference - - @property - def conversion_measure_reference(self) -> MeasureReference: - """Return the measure reference associated with the conversion measure.""" - return self.conversion_measure.measure_reference - class PydanticMetricTypeParams(HashableBaseModel): """Type params add additional context to certain metric types (the context depends on the metric type).""" @@ -180,12 +170,6 @@ class PydanticMetricTypeParams(HashableBaseModel): input_measures: List[PydanticMetricInputMeasure] = Field(default_factory=list) - @property - def conversion_type_params_or_error(self) -> PydanticConversionTypeParams: # noqa: D - if self.conversion_type_params is None: - raise ValueError("Expected conversion_type_params to be not None.") - return self.conversion_type_params - class PydanticMetric(HashableBaseModel, ModelWithMetadataParsing): """Describes a metric.""" diff --git a/dbt_semantic_interfaces/protocols/metric.py b/dbt_semantic_interfaces/protocols/metric.py index 0d907b9a..66b48dd1 100644 --- a/dbt_semantic_interfaces/protocols/metric.py +++ b/dbt_semantic_interfaces/protocols/metric.py @@ -118,9 +118,13 @@ def post_aggregation_reference(self) -> MetricReference: class ConstantPropertyInput(Protocol): - """Provides the constant property values for conversion metrics. + """Provides the constant property set for conversion metrics. - The specified property will be a reference of a dimension/entity. + Constant properties are additional elements linking a base event to a conversion event. + The specified properties will typically be a reference to a dimension or entity, and will be used + to join the base event to the final conversion event. Typical constant properties are things like + session keys (for services where conversions are measured within a user session), or secondary entities + (like a user/application pair for an app platform or a user/shop pair for a retail/online storefront platform). """ @property @@ -164,19 +168,7 @@ def calculation(self) -> ConversionCalculationType: @property @abstractmethod def window(self) -> Optional[MetricTimeWindow]: - """Maximum time range for finding successing conversion events.""" - pass - - @property - @abstractmethod - def base_measure_reference(self) -> MeasureReference: - """Return the measure reference associated with the base measure.""" - pass - - @property - @abstractmethod - def conversion_measure_reference(self) -> MeasureReference: - """Return the measure reference associated with the conversion measure.""" + """Maximum time range for finding successive conversion events.""" pass @property @@ -235,11 +227,6 @@ def metrics(self) -> Optional[Sequence[MetricInput]]: # noqa: D def conversion_type_params(self) -> Optional[ConversionTypeParams]: # noqa: D pass - @property - @abstractmethod - def conversion_type_params_or_error(self) -> ConversionTypeParams: # noqa: D - pass - class Metric(Protocol): """Describes a metric.""" diff --git a/dbt_semantic_interfaces/transformations/add_input_metric_measures.py b/dbt_semantic_interfaces/transformations/add_input_metric_measures.py index 844098db..bd09a50e 100644 --- a/dbt_semantic_interfaces/transformations/add_input_metric_measures.py +++ b/dbt_semantic_interfaces/transformations/add_input_metric_measures.py @@ -43,8 +43,10 @@ def _get_measures_for_metric( AddInputMetricMeasuresRule._get_measures_for_metric(semantic_manifest, input_metric.name) ) elif matched_metric.type is MetricType.CONVERSION: - measures.add(matched_metric.type_params.conversion_type_params_or_error.base_measure) - measures.add(matched_metric.type_params.conversion_type_params_or_error.conversion_measure) + conversion_type_params = matched_metric.type_params.conversion_type_params + assert conversion_type_params, "Conversion metric should have conversion_type_params." + measures.add(conversion_type_params.base_measure) + measures.add(conversion_type_params.conversion_measure) else: assert_values_exhausted(matched_metric.type) else: diff --git a/tests/test_implements_satisfy_protocols.py b/tests/test_implements_satisfy_protocols.py index cf0d7f69..03fb16ef 100644 --- a/tests/test_implements_satisfy_protocols.py +++ b/tests/test_implements_satisfy_protocols.py @@ -20,6 +20,7 @@ ) from dbt_semantic_interfaces.implementations.metadata import PydanticMetadata from dbt_semantic_interfaces.implementations.metric import ( + PydanticConversionTypeParams, PydanticMetric, PydanticMetricInput, PydanticMetricInputMeasure, @@ -204,7 +205,15 @@ def test_metric_protocol_derived(metric: PydanticMetric) -> None: # noqa: D builds( PydanticMetric, type=just(MetricType.CONVERSION), - type_params=builds(PydanticMetricTypeParams, metrics=lists(builds(PydanticMetricInput))), + type_params=builds( + PydanticMetricTypeParams, + conversion_type_params=builds( + PydanticConversionTypeParams, + base_measure=builds(PydanticMetricInputMeasure), + conversion_measure=builds(PydanticMetricInputMeasure), + entity=builds(str), + ), + ), expr=builds(str), ) )