Skip to content

Commit

Permalink
Rename BaseTimeGrainPattern -> MinimumTimeGrainPattern
Browse files Browse the repository at this point in the history
This change is intended to make it more clear what the difference is between MinimumTimeGrainPattern and MetricTimeDefaultTimeGranularityPattern, since 'base' might easily be confused with 'default'.
  • Loading branch information
courtneyholcomb committed Jul 12, 2024
1 parent ac43069 commit ac2b646
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ def filter_by_spec_patterns(self, spec_patterns: Sequence[SpecPattern]) -> Linka
"""
start_time = time.time()

# Spec patterns need all specs to match properly e.g. `BaseTimeGrainPattern`.
# Spec patterns need all specs to match properly e.g. `MinimumTimeGrainPattern`.
matching_specs: Sequence[InstanceSpec] = self.specs

for spec_pattern in spec_patterns:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
MetricFlowQueryResolutionIssueSet,
)
from metricflow_semantics.query.suggestion_generator import QueryItemSuggestionGenerator
from metricflow_semantics.specs.patterns.base_time_grain import BaseTimeGrainPattern
from metricflow_semantics.specs.patterns.minimum_time_grain import MinimumTimeGrainPattern
from metricflow_semantics.specs.patterns.no_group_by_metric import NoGroupByMetricPattern
from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern
from metricflow_semantics.specs.patterns.typed_patterns import TimeDimensionPattern
Expand Down Expand Up @@ -102,7 +102,7 @@ def resolve_matching_item_for_querying(
)

push_down_result = push_down_result.filter_candidates_by_pattern(
BaseTimeGrainPattern(),
MinimumTimeGrainPattern(),
)
logger.info(
f"Spec pattern:\n"
Expand Down Expand Up @@ -152,7 +152,7 @@ def resolve_matching_item_for_filters(

push_down_visitor = _PushDownGroupByItemCandidatesVisitor(
manifest_lookup=self._manifest_lookup,
source_spec_patterns=(spec_pattern, BaseTimeGrainPattern()),
source_spec_patterns=(spec_pattern, MinimumTimeGrainPattern()),
suggestion_generator=suggestion_generator,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@
ResolverInputForQuery,
ResolverInputForQueryLevelWhereFilterIntersection,
)
from metricflow_semantics.specs.patterns.base_time_grain import BaseTimeGrainPattern
from metricflow_semantics.specs.patterns.metric_time_pattern import MetricTimePattern
from metricflow_semantics.specs.patterns.minimum_time_grain import MinimumTimeGrainPattern
from metricflow_semantics.specs.patterns.none_date_part import NoneDatePartPattern
from metricflow_semantics.specs.query_param_implementations import DimensionOrEntityParameter, MetricParameter
from metricflow_semantics.specs.query_spec import MetricFlowQuerySpec
Expand Down Expand Up @@ -153,7 +153,7 @@ def _metric_time_granularity(time_dimension_specs: Sequence[TimeDimensionSpec])

for pattern_to_apply in (
MetricTimePattern(),
BaseTimeGrainPattern(),
MinimumTimeGrainPattern(),
NoneDatePartPattern(),
):
matching_specs = pattern_to_apply.match(matching_specs)
Expand All @@ -164,7 +164,7 @@ def _metric_time_granularity(time_dimension_specs: Sequence[TimeDimensionSpec])

assert (
len(time_dimension_specs) == 1
), f"Bug with BaseTimeGrainPattern - should have returned exactly 1 spec but got {time_dimension_specs}"
), f"Bug with MinimumTimeGrainPattern - should have returned exactly 1 spec but got {time_dimension_specs}"

return time_dimension_specs[0].time_granularity

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from metricflow_semantics.naming.naming_scheme import QueryItemNamingScheme
from metricflow_semantics.query.similarity import top_fuzzy_matches
from metricflow_semantics.specs.patterns.base_time_grain import BaseTimeGrainPattern
from metricflow_semantics.specs.patterns.minimum_time_grain import MinimumTimeGrainPattern
from metricflow_semantics.specs.patterns.no_group_by_metric import NoGroupByMetricPattern
from metricflow_semantics.specs.patterns.none_date_part import NoneDatePartPattern
from metricflow_semantics.specs.patterns.spec_pattern import SpecPattern
Expand All @@ -24,9 +24,9 @@ class QueryItemSuggestionGenerator:

# Adding these filters so that we don't get multiple suggestions that are similar, but with different
# grains. Some additional thought is needed to tweak this as the base grain may not be the best suggestion.
FILTER_ITEM_CANDIDATE_FILTERS: Tuple[SpecPattern, ...] = (BaseTimeGrainPattern(), NoneDatePartPattern())
FILTER_ITEM_CANDIDATE_FILTERS: Tuple[SpecPattern, ...] = (MinimumTimeGrainPattern(), NoneDatePartPattern())
GROUP_BY_ITEM_CANDIDATE_FILTERS: Tuple[SpecPattern, ...] = (
BaseTimeGrainPattern(),
MinimumTimeGrainPattern(),
NoneDatePartPattern(),
NoGroupByMetricPattern(),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from metricflow_semantics.specs.spec_set import group_specs_by_type


class BaseTimeGrainPattern(SpecPattern):
class MinimumTimeGrainPattern(SpecPattern):
"""A pattern that matches linkable specs, but for time dimension specs, only the one with the finest grain.
e.g.
Expand Down Expand Up @@ -63,7 +63,9 @@ def match(self, candidate_specs: Sequence[InstanceSpec]) -> Sequence[InstanceSpe
metric_time_specs = MetricTimePattern().match(candidate_specs)
other_specs = tuple(spec for spec in candidate_specs if spec not in metric_time_specs)

return other_specs + tuple(BaseTimeGrainPattern(only_apply_for_metric_time=False).match(metric_time_specs))
return other_specs + tuple(
MinimumTimeGrainPattern(only_apply_for_metric_time=False).match(metric_time_specs)
)

spec_set = group_specs_by_type(candidate_specs)

Expand Down

0 comments on commit ac2b646

Please sign in to comment.