Skip to content

Commit

Permalink
Cleanup (#1161)
Browse files Browse the repository at this point in the history
* Move logic for directly linkable metrics to _get_joined_elements

* Get more specific about avoiding cycle creation

* Update documentation
  • Loading branch information
courtneyholcomb authored Apr 30, 2024
1 parent 3f2dbc5 commit 8237b7f
Showing 1 changed file with 15 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,12 @@ def get_joinable_metrics_for_semantic_model(
"""Get the set of linkable metrics that can be joined to this semantic model."""
properties = frozenset({LinkableElementProperty.METRIC, LinkableElementProperty.JOINED})
if using_join_path:
assert (
semantic_model.reference == using_join_path.last_semantic_model_reference
), "Last join path element should match semantic model when building LinkableMetrics."
assert semantic_model.reference == using_join_path.last_semantic_model_reference, (
"Last join path element should match semantic model when building LinkableMetrics. "
f"Got semantic model: {semantic_model.reference.semantic_model_name}; "
f"last join path element: {using_join_path.last_semantic_model_reference.semantic_model_name}",
)
# TODO: confirm that this is what we internally consider multi-hop
properties = properties.union(frozenset({LinkableElementProperty.MULTI_HOP}))

path_key_to_linkable_metrics: Dict[ElementPathKey, Tuple[LinkableMetric, ...]] = {}
Expand Down Expand Up @@ -253,6 +256,7 @@ def _get_elements_in_semantic_model(self, semantic_model: SemanticModel) -> Link
"""Gets the elements in the semantic model, without requiring any joins.
Elements related to metric_time are handled separately in _get_metric_time_elements().
Linkable metrics are not considered local to the semantic model since they always require a join.
"""
linkable_dimensions = []
linkable_entities = []
Expand Down Expand Up @@ -446,6 +450,8 @@ def _get_metric_time_elements(self, measure_reference: Optional[MeasureReference

def _get_joined_elements(self, measure_semantic_model: SemanticModel) -> LinkableElementSet:
"""Get the elements that can be generated by joining other models to the given model."""
directly_linkable_metrics = self.get_joinable_metrics_for_semantic_model(semantic_model=measure_semantic_model)

# Create single-hop elements
join_paths = []
for entity in measure_semantic_model.entities:
Expand Down Expand Up @@ -490,7 +496,9 @@ def _get_joined_elements(self, measure_semantic_model: SemanticModel) -> Linkabl
)
join_paths = new_join_paths

return LinkableElementSet.merge_by_path_key((single_hop_elements, multi_hop_elements))
return LinkableElementSet.merge_by_path_key(
(directly_linkable_metrics, single_hop_elements, multi_hop_elements)
)

def _get_linkable_element_set_for_measure(
self,
Expand All @@ -502,17 +510,11 @@ def _get_linkable_element_set_for_measure(
measure_semantic_model = self._get_semantic_model_for_measure(measure_reference)

elements_in_semantic_model = self._get_elements_in_semantic_model(measure_semantic_model)
metrics_linked_to_semantic_model = self.get_joinable_metrics_for_semantic_model(measure_semantic_model)
metric_time_elements = self._get_metric_time_elements(measure_reference)
joined_elements = self._get_joined_elements(measure_semantic_model)

return LinkableElementSet.merge_by_path_key(
(
elements_in_semantic_model,
metrics_linked_to_semantic_model,
metric_time_elements,
joined_elements,
)
(elements_in_semantic_model, metric_time_elements, joined_elements)
).filter(
with_any_of=with_any_of,
without_any_of=without_any_of,
Expand Down Expand Up @@ -655,8 +657,8 @@ def create_linkable_element_set_from_join_path(
raise RuntimeError(f"Unhandled type: {dimension_type}")

for entity in semantic_model.entities:
# Avoid creating "booking_id__booking_id"
if entity.reference != join_path.last_entity_link:
# Avoid creating an entity cycle
if entity.reference not in join_path.entity_links:
linkable_entities.append(
LinkableEntity(
semantic_model_origin=semantic_model.reference,
Expand Down

0 comments on commit 8237b7f

Please sign in to comment.