Skip to content

Commit

Permalink
Remove include_base_grain property and filter elements properly instead
Browse files Browse the repository at this point in the history
  • Loading branch information
courtneyholcomb committed Sep 24, 2024
1 parent 4aa9095 commit 84b90f5
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 24 deletions.
11 changes: 2 additions & 9 deletions metricflow/dataflow/builder/dataflow_plan_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -798,11 +798,8 @@ def _build_plan_for_distinct_values(

for time_dimension_spec in required_linkable_specs.time_dimension_specs:
if time_dimension_spec.time_granularity.is_custom_granularity:
include_base_grain = time_dimension_spec.with_base_grain in required_linkable_specs.time_dimension_specs
output_node = JoinToCustomGranularityNode.create(
parent_node=output_node,
time_dimension_spec=time_dimension_spec,
include_base_grain=include_base_grain,
parent_node=output_node, time_dimension_spec=time_dimension_spec
)

if len(query_level_filter_specs) > 0:
Expand Down Expand Up @@ -1583,11 +1580,8 @@ def _build_aggregated_measure_from_measure_source_node(

for time_dimension_spec in required_linkable_specs.time_dimension_specs:
if time_dimension_spec.time_granularity.is_custom_granularity:
include_base_grain = time_dimension_spec.with_base_grain in required_linkable_specs.time_dimension_specs
unaggregated_measure_node = JoinToCustomGranularityNode.create(
parent_node=unaggregated_measure_node,
time_dimension_spec=time_dimension_spec,
include_base_grain=include_base_grain,
parent_node=unaggregated_measure_node, time_dimension_spec=time_dimension_spec
)

# If time constraint was previously adjusted for cumulative window or grain, apply original time constraint
Expand Down Expand Up @@ -1649,7 +1643,6 @@ def _build_aggregated_measure_from_measure_source_node(
# show up in the final result.
#
# e.g. for "bookings" by "ds" where "is_instant", "is_instant" should not be in the results.
# TODO: filter out base grain here
pre_aggregate_node = FilterElementsNode.create(
parent_node=pre_aggregate_node,
include_specs=InstanceSpecSet(measure_specs=(measure_spec,)).merge(
Expand Down
18 changes: 3 additions & 15 deletions metricflow/dataflow/nodes/join_to_custom_granularity.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,9 @@ class JoinToCustomGranularityNode(DataflowPlanNode, ABC):
Args:
time_dimension_spec: The time dimension spec with a custom granularity that will be satisfied by this node.
include_base_grain: Bool that indicates if a spec with the custom granularity's base grain
should be included in the node's output dataset. This is needed when the same time dimension is requested
twice in one query, with both a custom grain and that custom grain's base grain.
"""

time_dimension_spec: TimeDimensionSpec
include_base_grain: bool

def __post_init__(self) -> None: # noqa: D105
assert (
Expand All @@ -34,11 +30,9 @@ def __post_init__(self) -> None: # noqa: D105

@staticmethod
def create( # noqa: D102
parent_node: DataflowPlanNode, time_dimension_spec: TimeDimensionSpec, include_base_grain: bool
parent_node: DataflowPlanNode, time_dimension_spec: TimeDimensionSpec
) -> JoinToCustomGranularityNode:
return JoinToCustomGranularityNode(
parent_nodes=(parent_node,), time_dimension_spec=time_dimension_spec, include_base_grain=include_base_grain
)
return JoinToCustomGranularityNode(parent_nodes=(parent_node,), time_dimension_spec=time_dimension_spec)

@classmethod
def id_prefix(cls) -> IdPrefix: # noqa: D102
Expand All @@ -55,19 +49,14 @@ def description(self) -> str: # noqa: D102
def displayed_properties(self) -> Sequence[DisplayedProperty]: # noqa: D102
return tuple(super().displayed_properties) + (
DisplayedProperty("time_dimension_spec", self.time_dimension_spec),
DisplayedProperty("include_base_grain", self.include_base_grain),
)

@property
def parent_node(self) -> DataflowPlanNode: # noqa: D102
return self.parent_nodes[0]

def functionally_identical(self, other_node: DataflowPlanNode) -> bool: # noqa: D102
return (
isinstance(other_node, self.__class__)
and other_node.time_dimension_spec == self.time_dimension_spec
and other_node.include_base_grain == self.include_base_grain
)
return isinstance(other_node, self.__class__) and other_node.time_dimension_spec == self.time_dimension_spec

def with_new_parents( # noqa: D102
self, new_parent_nodes: Sequence[DataflowPlanNode]
Expand All @@ -76,5 +65,4 @@ def with_new_parents( # noqa: D102
return JoinToCustomGranularityNode.create(
parent_node=new_parent_nodes[0],
time_dimension_spec=self.time_dimension_spec,
include_base_grain=self.include_base_grain,
)

0 comments on commit 84b90f5

Please sign in to comment.