Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change resolved_primary_entity helper signature #1485

Merged
merged 1 commit into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from typing import Optional, Sequence
from typing import Sequence

from dbt_semantic_interfaces.protocols import Dimension
from dbt_semantic_interfaces.protocols.entity import Entity
Expand Down Expand Up @@ -31,8 +31,12 @@ def get_entity_from_semantic_model(
)

@staticmethod
def resolved_primary_entity(semantic_model: SemanticModel) -> Optional[EntityReference]:
"""Return the primary entity for dimensions in the model."""
def resolved_primary_entity(semantic_model: SemanticModel) -> EntityReference:
"""Return the primary entity for dimensions in the model.

Semantic models with measures or dimensions should have a primary entity as enforced by semantic manifest
validation.
"""
primary_entity_reference = semantic_model.primary_entity_reference

entities_with_type_primary = tuple(
Expand All @@ -51,7 +55,7 @@ def resolved_primary_entity(semantic_model: SemanticModel) -> Optional[EntityRef
if len(entities_with_type_primary) > 0:
return entities_with_type_primary[0].reference

return None
raise ValueError(f"No primary entity found in {semantic_model.reference=}")

@staticmethod
def entity_links_for_local_elements(semantic_model: SemanticModel) -> Sequence[EntityReference]:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

from metricflow_semantics.errors.error_classes import InvalidSemanticModelError
from metricflow_semantics.mf_logging.lazy_formattable import LazyFormat
from metricflow_semantics.mf_logging.pretty_print import mf_pformat
from metricflow_semantics.model.semantics.element_group import ElementGrouper
from metricflow_semantics.model.semantics.semantic_model_helper import SemanticModelHelper
from metricflow_semantics.model.spec_converters import MeasureConverter
Expand Down Expand Up @@ -198,9 +197,6 @@ def _add_semantic_model(self, semantic_model: SemanticModel) -> None:
f"Aggregation time dimension does not have a time granularity set: {agg_time_dimension}"
)

# TODO: do we need this here? This should be handled in validations
self.get_primary_entity_else_error(semantic_model)

self._semantic_model_to_aggregation_time_dimensions[semantic_model.reference].add_value(
key=TimeDimensionReference(
element_name=agg_time_dimension.name,
Expand Down Expand Up @@ -246,22 +242,6 @@ def _add_semantic_model(self, semantic_model: SemanticModel) -> None:

self._semantic_model_reference_to_semantic_model[semantic_model.reference] = semantic_model

def get_primary_entity_else_error(self, semantic_model: SemanticModel) -> EntityReference:
"""Get primary entity from semantic model and error if it doesn't exist.

If there are dimensions in the semantic model, there must be a primary entity. If there are measures, we can
also assume there must be a primary entity because measures are required to have an `agg_time_dimension`
defined in the same semantic model.
"""
# TODO: Move me.
primary_entity = SemanticModelHelper.resolved_primary_entity(semantic_model)
if primary_entity is None:
raise RuntimeError(
f"The semantic model should have a primary entity since there are dimensions, but it does not. "
f"Semantic model is:\n{mf_pformat(semantic_model)}"
)
return primary_entity

def get_aggregation_time_dimensions_with_measures(
self, semantic_model_reference: SemanticModelReference
) -> ElementGrouper[TimeDimensionReference, MeasureSpec]:
Expand Down Expand Up @@ -310,10 +290,6 @@ def _get_agg_time_dimension_specs_for_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)
assert entity_link is not None, (
f"Expected semantic model {semantic_model} to have a primary entity since it has a "
"measure requiring an agg_time_dimension, but found none.",
)
return TimeDimensionSpec.generate_possible_specs_for_time_dimension(
time_dimension_reference=agg_time_dimension,
entity_links=(entity_link,),
Expand Down
2 changes: 1 addition & 1 deletion metricflow/engine/metricflow_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ def list_dimensions(self) -> List[Dimension]: # noqa: D102
pydantic_dimension=SemanticModelHelper.get_dimension_from_semantic_model(
semantic_model=semantic_model, dimension_reference=dimension_reference
),
entity_links=(semantic_model_lookup.get_primary_entity_else_error(semantic_model),),
entity_links=(SemanticModelHelper.resolved_primary_entity(semantic_model),),
)
)

Expand Down
Loading