From 647a8bed25b4696c93531776ba1d16a430334da3 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Fri, 8 Nov 2024 17:54:51 -0800 Subject: [PATCH] Remove unused code (#1497) This PR removes code that was made redundant / tests moved with: https://github.com/dbt-labs/metricflow/pull/1486 https://github.com/dbt-labs/metricflow/pull/1487 --- .../semantics/semantic_model_container.py | 28 ----- .../model/semantics/semantic_model_lookup.py | 118 +----------------- .../model/test_semantic_model_container.py | 31 ----- 3 files changed, 2 insertions(+), 175 deletions(-) delete mode 100644 metricflow-semantics/metricflow_semantics/model/semantics/semantic_model_container.py diff --git a/metricflow-semantics/metricflow_semantics/model/semantics/semantic_model_container.py b/metricflow-semantics/metricflow_semantics/model/semantics/semantic_model_container.py deleted file mode 100644 index cd81c99c49..0000000000 --- a/metricflow-semantics/metricflow_semantics/model/semantics/semantic_model_container.py +++ /dev/null @@ -1,28 +0,0 @@ -from __future__ import annotations - -from abc import ABC, abstractmethod -from typing import Generic, List, TypeVar - -T = TypeVar("T") - - -class SemanticModelContainer(ABC, Generic[T]): # noqa: D101 - @abstractmethod - def get(self, semantic_model_name: str) -> T: # noqa: D102 - pass - - @abstractmethod - def values(self) -> List[T]: # noqa: D102 - pass - - @abstractmethod - def keys(self) -> List[str]: # noqa: D102 - pass - - @abstractmethod - def __contains__(self, item: str) -> bool: # noqa: D105 - pass - - @abstractmethod - def put(self, key: str, value: T) -> None: # noqa: D102 - pass diff --git a/metricflow-semantics/metricflow_semantics/model/semantics/semantic_model_lookup.py b/metricflow-semantics/metricflow_semantics/model/semantics/semantic_model_lookup.py index f122f6f5c2..fd94bf659b 100644 --- a/metricflow-semantics/metricflow_semantics/model/semantics/semantic_model_lookup.py +++ b/metricflow-semantics/metricflow_semantics/model/semantics/semantic_model_lookup.py @@ -4,9 +4,7 @@ from functools import cached_property from typing import Dict, List, Optional, Sequence, Set, Tuple -from dbt_semantic_interfaces.protocols.dimension import Dimension from dbt_semantic_interfaces.protocols.entity import Entity -from dbt_semantic_interfaces.protocols.measure import Measure from dbt_semantic_interfaces.protocols.semantic_manifest import SemanticManifest from dbt_semantic_interfaces.protocols.semantic_model import SemanticModel from dbt_semantic_interfaces.references import ( @@ -17,14 +15,13 @@ SemanticModelReference, TimeDimensionReference, ) -from dbt_semantic_interfaces.type_enums import AggregationType, DimensionType, TimeGranularity +from dbt_semantic_interfaces.type_enums import DimensionType from metricflow_semantics.errors.error_classes import InvalidSemanticModelError from metricflow_semantics.mf_logging.lazy_formattable import LazyFormat from metricflow_semantics.model.semantics.dimension_lookup import DimensionLookup from metricflow_semantics.model.semantics.element_group import ElementGrouper from metricflow_semantics.model.semantics.measure_lookup import MeasureLookup -from metricflow_semantics.model.semantics.semantic_model_helper import SemanticModelHelper from metricflow_semantics.model.spec_converters import MeasureConverter from metricflow_semantics.naming.linkable_spec_name import StructuredLinkableSpecName from metricflow_semantics.specs.dimension_spec import DimensionSpec @@ -32,7 +29,7 @@ from metricflow_semantics.specs.instance_spec import LinkableInstanceSpec from metricflow_semantics.specs.measure_spec import MeasureSpec from metricflow_semantics.specs.non_additive_dimension_spec import NonAdditiveDimensionSpec -from metricflow_semantics.specs.time_dimension_spec import DEFAULT_TIME_GRANULARITY, TimeDimensionSpec +from metricflow_semantics.specs.time_dimension_spec import TimeDimensionSpec from metricflow_semantics.time.granularity import ExpandedTimeGranularity logger = logging.getLogger(__name__) @@ -49,8 +46,6 @@ def __init__(self, model: SemanticManifest, custom_granularities: Dict[str, Expa """ self._custom_granularities = custom_granularities self._measure_index: Dict[MeasureReference, SemanticModel] = {} - self._measure_aggs: Dict[MeasureReference, AggregationType] = {} - self._measure_agg_time_dimension: Dict[MeasureReference, TimeDimensionReference] = {} self._measure_non_additive_dimension_specs: Dict[MeasureReference, NonAdditiveDimensionSpec] = {} self._dimension_index: Dict[DimensionReference, List[SemanticModel]] = {} self._entity_index: Dict[EntityReference, List[SemanticModel]] = {} @@ -67,12 +62,6 @@ def __init__(self, model: SemanticManifest, custom_granularities: Dict[str, Expa for semantic_model in sorted_semantic_models: self._add_semantic_model(semantic_model) - # Cache for defined time granularity. - self._time_dimension_to_defined_time_granularity: Dict[TimeDimensionReference, TimeGranularity] = {} - - # Cache for agg. time dimension for measure. - self._measure_reference_to_agg_time_dimension_specs: Dict[MeasureReference, Sequence[TimeDimensionSpec]] = {} - self._measure_lookup = MeasureLookup(sorted_semantic_models, custom_granularities) self._dimension_lookup = DimensionLookup(sorted_semantic_models) @@ -85,27 +74,6 @@ def get_dimension_references(self) -> Sequence[DimensionReference]: """Retrieve all dimension references from the collection of semantic models.""" return tuple(self._dimension_index.keys()) - def get_dimension(self, dimension_reference: DimensionReference) -> Dimension: - """Retrieves a full dimension object by name.""" - # If the reference passed is a TimeDimensionReference, convert to DimensionReference. - dimension_reference = DimensionReference(dimension_reference.element_name) - - semantic_models = self._dimension_index.get(dimension_reference) - if not semantic_models: - raise ValueError( - f"Could not find dimension with name '{dimension_reference.element_name}' in configured semantic models" - ) - - return SemanticModelHelper.get_dimension_from_semantic_model( - # Dimension object should match across semantic models, so just use the first semantic model. - semantic_model=semantic_models[0], - dimension_reference=dimension_reference, - ) - - def get_time_dimension(self, time_dimension_reference: TimeDimensionReference) -> Dimension: - """Retrieves a full dimension object by name.""" - return self.get_dimension(dimension_reference=time_dimension_reference.dimension_reference) - @property def measure_references(self) -> Sequence[MeasureReference]: """Return all measure references from the collection of semantic models.""" @@ -119,35 +87,10 @@ def non_additive_dimension_specs_by_measure(self) -> Dict[MeasureReference, NonA """ return self._measure_non_additive_dimension_specs - def get_measure(self, measure_reference: MeasureReference) -> Measure: - """Retrieve the measure model object associated with the measure reference.""" - if measure_reference not in self._measure_index: - raise ValueError(f"Could not find measure with name ({measure_reference}) in configured semantic models") - - return SemanticModelHelper.get_measure_from_semantic_model( - semantic_model=self.get_semantic_model_for_measure(measure_reference), measure_reference=measure_reference - ) - def get_entity_references(self) -> Sequence[EntityReference]: """Retrieve all entity references from the collection of semantic models.""" return list(self._entity_index.keys()) - def get_semantic_model_for_measure(self, measure_reference: MeasureReference) -> SemanticModel: # noqa: D102 - semantic_model = self._measure_index.get(measure_reference) - assert semantic_model, ( - f"Semantic model not found for measure: {repr(measure_reference)}. " - f"This indicates either internal misconfiguration or that the measure does not exist." - ) - return semantic_model - - def get_agg_time_dimension_for_measure(self, measure_reference: MeasureReference) -> TimeDimensionReference: - """Retrieves the aggregate time dimension that is associated with the measure reference. - - This is the time dimension along which the measure will be aggregated when a metric built on this measure - is queried with metric_time. - """ - return self._measure_agg_time_dimension[measure_reference] - def get_entity_in_semantic_model(self, ref: SemanticModelElementReference) -> Optional[Entity]: """Retrieve the entity matching the element -> semantic model mapping, if any.""" semantic_model = self.get_by_reference(ref.semantic_model_reference) @@ -171,13 +114,6 @@ def _add_semantic_model(self, semantic_model: SemanticModel) -> None: if semantic_model.reference in self._semantic_model_reference_to_semantic_model: errors.append(f"Semantic model {semantic_model.reference} already added.") - for measure in semantic_model.measures: - if measure.reference in self._measure_aggs and self._measure_aggs[measure.reference] != measure.agg: - errors.append( - f"Conflicting aggregation (agg) for measure {measure.reference}. Currently registered as " - f"{self._measure_aggs[measure.reference]} but got {measure.agg}." - ) - if len(errors) > 0: raise InvalidSemanticModelError(f"Error adding {semantic_model.reference}. Got errors: {errors}") @@ -186,7 +122,6 @@ def _add_semantic_model(self, semantic_model: SemanticModel) -> None: ]() for measure in semantic_model.measures: - self._measure_aggs[measure.reference] = measure.agg self._measure_index[measure.reference] = semantic_model agg_time_dimension_reference = semantic_model.checked_agg_time_dimension_for_measure(measure.reference) @@ -215,9 +150,6 @@ def _add_semantic_model(self, semantic_model: SemanticModel) -> None: ), value=MeasureConverter.convert_to_measure_spec(measure=measure), ) - self._measure_agg_time_dimension[measure.reference] = TimeDimensionReference( - element_name=agg_time_dimension.name - ) if measure.non_additive_dimension: non_additive_dimension_spec = NonAdditiveDimensionSpec( @@ -284,52 +216,6 @@ def get_element_spec_for_name(self, element_name: str) -> LinkableInstanceSpec: else: raise ValueError(f"Unable to find linkable element {element_name} in manifest") - def get_agg_time_dimension_specs_for_measure( - self, measure_reference: MeasureReference - ) -> Sequence[TimeDimensionSpec]: - """Get the agg time dimension specs that can be used in place of metric time for this measure.""" - result = self._measure_reference_to_agg_time_dimension_specs.get(measure_reference) - if result is not None: - return result - - result = self._get_agg_time_dimension_specs_for_measure(measure_reference) - self._measure_reference_to_agg_time_dimension_specs[measure_reference] = result - return result - - def _get_agg_time_dimension_specs_for_measure( - self, measure_reference: MeasureReference - ) -> Sequence[TimeDimensionSpec]: - agg_time_dimension = self.get_agg_time_dimension_for_measure(measure_reference) - # A measure's agg_time_dimension is required to be in the same semantic model as the measure, - # so we can assume the same semantic model for both measure and dimension. - semantic_model = self.get_semantic_model_for_measure(measure_reference) - entity_link = SemanticModelHelper.resolved_primary_entity(semantic_model) - return TimeDimensionSpec.generate_possible_specs_for_time_dimension( - time_dimension_reference=agg_time_dimension, - entity_links=(entity_link,), - custom_granularities=self._custom_granularities, - ) - - def get_defined_time_granularity(self, time_dimension_reference: TimeDimensionReference) -> TimeGranularity: - """Time granularity from the time dimension's YAML definition. If not set, defaults to DAY.""" - result = self._time_dimension_to_defined_time_granularity.get(time_dimension_reference) - - if result is not None: - return result - - result = self._get_defined_time_granularity(time_dimension_reference) - self._time_dimension_to_defined_time_granularity[time_dimension_reference] = result - return result - - def _get_defined_time_granularity(self, time_dimension_reference: TimeDimensionReference) -> TimeGranularity: - time_dimension = self.get_dimension(time_dimension_reference) - - defined_time_granularity = DEFAULT_TIME_GRANULARITY - if time_dimension.type_params and time_dimension.type_params.time_granularity: - defined_time_granularity = time_dimension.type_params.time_granularity - - return defined_time_granularity - @property def measure_lookup(self) -> MeasureLookup: # noqa: D102 return self._measure_lookup diff --git a/metricflow-semantics/tests_metricflow_semantics/model/test_semantic_model_container.py b/metricflow-semantics/tests_metricflow_semantics/model/test_semantic_model_container.py index 907c5d3bb9..df753ceafa 100644 --- a/metricflow-semantics/tests_metricflow_semantics/model/test_semantic_model_container.py +++ b/metricflow-semantics/tests_metricflow_semantics/model/test_semantic_model_container.py @@ -77,28 +77,6 @@ def test_get_names( # noqa: D103 ) -def test_get_elements(semantic_model_lookup: SemanticModelLookup) -> None: # noqa: D103 - for dimension_reference in semantic_model_lookup.get_dimension_references(): - assert ( - semantic_model_lookup.get_dimension(dimension_reference=dimension_reference).reference - == dimension_reference - ) - for measure_reference in semantic_model_lookup.measure_references: - measure_reference = MeasureReference(element_name=measure_reference.element_name) - assert semantic_model_lookup.get_measure(measure_reference=measure_reference).reference == measure_reference - - -def test_get_semantic_model_for_measure(semantic_model_lookup: SemanticModelLookup) -> None: # noqa: D103 - bookings_source = semantic_model_lookup.get_semantic_model_for_measure(MeasureReference(element_name="bookings")) - assert bookings_source.name == "bookings_source" - - views_source = semantic_model_lookup.get_semantic_model_for_measure(MeasureReference(element_name="views")) - assert views_source.name == "views_source" - - listings_source = semantic_model_lookup.get_semantic_model_for_measure(MeasureReference(element_name="listings")) - assert listings_source.name == "listings_latest" - - def test_local_linked_elements_for_metric( # noqa: D103 request: FixtureRequest, mf_test_configuration: MetricFlowTestConfiguration, metric_lookup: MetricLookup ) -> None: @@ -239,12 +217,3 @@ def test_get_valid_agg_time_dimensions_for_metric( # noqa: D103 assert metric_agg_time_dim.reference == measure_agg_time_dims[0] else: assert len(metric_agg_time_dims) == 0 - - -def test_get_agg_time_dimension_specs_for_measure(semantic_model_lookup: SemanticModelLookup) -> None: # noqa: D103 - for measure_name in ["bookings", "views", "listings"]: - measure_reference = MeasureReference(measure_name) - agg_time_dim_specs = semantic_model_lookup.get_agg_time_dimension_specs_for_measure(measure_reference) - agg_time_dim_reference = semantic_model_lookup.get_agg_time_dimension_for_measure(measure_reference) - for spec in agg_time_dim_specs: - assert spec.reference == agg_time_dim_reference