Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
courtneyholcomb committed Sep 18, 2023
1 parent 8a950d4 commit 1015b5f
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 24 deletions.
7 changes: 6 additions & 1 deletion metricflow/naming/linkable_spec_name.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,13 @@ def from_name(qualified_name: str) -> StructuredLinkableSpecName:
if len(name_parts) == 1:
return StructuredLinkableSpecName(entity_link_names=(), element_name=name_parts[0])

for date_part in DatePart:
if name_parts[-1] == StructuredLinkableSpecName.date_part_suffix(date_part=date_part):
raise ValueError(
"Dunder syntax not supported for querying date_part. Use `group_by` object syntax instead."
)

associated_granularity = None
granularity: TimeGranularity
for granularity in TimeGranularity:
if name_parts[-1] == granularity.value:
associated_granularity = granularity
Expand Down
26 changes: 5 additions & 21 deletions metricflow/query/query_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,25 +289,6 @@ def _construct_metric_specs_for_query(
)
return tuple(metric_specs)

def _get_group_by_names(
self, group_by_names: Optional[Sequence[str]], group_by: Optional[Sequence[QueryParameter]]
) -> Sequence[str]:
assert not (
group_by_names and group_by
), "Both group_by_names and group_by were set, but if a group by is specified you should only use one of these!"
return (
group_by_names
if group_by_names
else [
StructuredLinkableSpecName(
entity_link_names=(), element_name=g.name, time_granularity=g.grain, date_part=g.date_part
).qualified_name
for g in group_by
]
if group_by
else []
)

def _get_metric_names(
self, metric_names: Optional[Sequence[str]], metrics: Optional[Sequence[QueryInterfaceMetric]]
) -> Sequence[str]:
Expand Down Expand Up @@ -551,7 +532,8 @@ def _validate_date_part(
if time_dimension_spec.date_part.to_int() < time_dimension_spec.time_granularity.to_int():
raise RequestTimeGranularityException(
f"Date part {time_dimension_spec.date_part.name} is not compatible with time granularity "
f"{time_dimension_spec.time_granularity.name}."
f"{time_dimension_spec.time_granularity.name}. Compatible granularities include: "
f"{[granularity.name for granularity in time_dimension_spec.date_part.compatible_granularities]}"
)
if date_part_requested:
for metric_reference in metric_references:
Expand Down Expand Up @@ -686,7 +668,9 @@ def _parse_linkable_elements(
linkable_elements: Optional[Sequence[QueryParameter]] = None,
) -> QueryTimeLinkableSpecSet:
"""Convert the linkable spec names into the respective specification objects."""
assert not (qualified_linkable_names and linkable_elements)
assert not (
qualified_linkable_names and linkable_elements
), "Both group_by_names and group_by were set, but if a group by is specified you should only use one of these!"

structured_names: List[StructuredLinkableSpecName] = []
if qualified_linkable_names:
Expand Down
2 changes: 0 additions & 2 deletions metricflow/specs/where_filter_time_dimension.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,4 @@ def _convert_to_time_dimension_spec(
element_name=parameter_set.time_dimension_reference.element_name,
entity_links=parameter_set.entity_path,
time_granularity=parameter_set.time_granularity,
# TODO: add date_part to TimeDimensionCallParameterSet in DSI
# date_part=parameter_set.date_part,
)
6 changes: 6 additions & 0 deletions metricflow/time/date_part.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

from enum import Enum
from typing import List

from dbt_semantic_interfaces.enum_extension import assert_values_exhausted
from dbt_semantic_interfaces.type_enums.time_granularity import TimeGranularity
Expand Down Expand Up @@ -38,3 +39,8 @@ def to_int(self) -> int:
return TimeGranularity.YEAR.to_int()
else:
assert_values_exhausted(self)

@property
def compatible_granularities(self) -> List[TimeGranularity]:
"""Granularities that can be queried with this date part."""
return [granularity for granularity in TimeGranularity if granularity.to_int() >= self.to_int()]

0 comments on commit 1015b5f

Please sign in to comment.