Skip to content

Commit

Permalink
Fix pretty print bug for sets containing enums (#1182)
Browse files Browse the repository at this point in the history 
### Description
The recent change to allow & sort sets in MF pretty printing was hiding
a sneaky error when pretty printing objects containing enums. Enums
can't be sorted without a key. This would only show up in logs that call
`mf_pformat()`, since none of our test snapshots appear to contain enums
- hence why it wasn't caught in CI.
Here's an example of a logging error triggered by query parsing that was
being buried by the try/except in `mf_pformat()`:
`TypeError: '<' not supported between instances of
'LinkableElementProperty' and 'LinkableElementProperty'`
  • Loading branch information
@courtneyholcomb
courtneyholcomb authored May 7, 2024
1 parent 62652c2 commit 249a23f
Show file tree
Hide file tree
Showing 31 changed files with 54 additions and 54 deletions.
4 changes: 2 additions & 2 deletions metricflow-semantics/metricflow_semantics/mf_logging/pretty_print.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def _handle_sequence_obj(
"""Pretty prints a sequence object i.e. list or tuple.
Args:
list_like_obj: A list or a tuple.
list_like_obj: A list, tuple, set, or frozenset.
remaining_line_width: If specified, try to make the string representation <= this many columns wide.
Returns:
Expand All @@ -60,7 +60,7 @@ def _handle_sequence_obj(
elif isinstance(list_like_obj, set) or isinstance(list_like_obj, frozenset):
left_enclose_str = f"{type(list_like_obj).__name__}("
right_enclose_str = ")"
list_like_obj = sorted(list_like_obj)
list_like_obj = sorted(self._handle_any_obj(obj, None) for obj in list_like_obj)
else:
raise RuntimeError(f"Unhandled type: {type(list_like_obj)}")

Expand Down
2 changes: 1 addition & 1 deletion ...n/test_ambiguous_metric_time_in_query_filter__accumulate_last_2_months_metric__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ GroupByItemResolution(
),
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(METRIC_TIME,),
properties=frozenset('METRIC_TIME',),
time_granularity=MONTH,
),
),
Expand Down
4 changes: 2 additions & 2 deletions ...metric_time_in_query_filter__derived_metric_with_different_parent_time_grains__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ GroupByItemResolution(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down Expand Up @@ -66,7 +66,7 @@ GroupByItemResolution(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=YEAR,
),
Expand Down
2 changes: 1 addition & 1 deletion ...uous_metric_time_in_query_filter__derived_metric_with_same_parent_time_grains__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ GroupByItemResolution(
),
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(METRIC_TIME,),
properties=frozenset('METRIC_TIME',),
time_granularity=MONTH,
),
),
Expand Down
4 changes: 2 additions & 2 deletions ...est_ambiguous_metric_time_in_query_filter__metrics_with_different_time_grains__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ GroupByItemResolution(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down Expand Up @@ -61,7 +61,7 @@ GroupByItemResolution(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=YEAR,
),
Expand Down
2 changes: 1 addition & 1 deletion ...ion/test_ambiguous_metric_time_in_query_filter__metrics_with_same_time_grains__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ GroupByItemResolution(
),
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(METRIC_TIME,),
properties=frozenset('METRIC_TIME',),
time_granularity=MONTH,
),
),
Expand Down
2 changes: 1 addition & 1 deletion .../GroupByItemResolution/test_ambiguous_metric_time_in_query_filter__no_metrics__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ GroupByItemResolution(
LinkableDimension(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(METRIC_TIME,),
properties=frozenset('METRIC_TIME',),
time_granularity=DAY,
),
),
Expand Down
2 changes: 1 addition & 1 deletion ...oupByItemResolution/test_ambiguous_metric_time_in_query_filter__simple_metric__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ GroupByItemResolution(
),
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(METRIC_TIME,),
properties=frozenset('METRIC_TIME',),
time_granularity=MONTH,
),
),
Expand Down
4 changes: 2 additions & 2 deletions ...py/str/test_filter_resolution_for_derived_metrics_with_common_filtered_metric__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down Expand Up @@ -94,7 +94,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down
4 changes: 2 additions & 2 deletions ...hots/test_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_filter__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down Expand Up @@ -95,7 +95,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=YEAR,
),
Expand Down
4 changes: 2 additions & 2 deletions ...est_spec_lookup.py/str/test_filter_resolution_for_invalid_metric_input_filter__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down Expand Up @@ -98,7 +98,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=YEAR,
),
Expand Down
2 changes: 1 addition & 1 deletion ...pshots/test_spec_lookup.py/str/test_filter_resolution_for_valid_metric_filter__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down
2 changes: 1 addition & 1 deletion .../test_spec_lookup.py/str/test_filter_resolution_for_valid_metric_input_filter__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down
2 changes: 1 addition & 1 deletion ...ec_lookup.py/str/test_filter_spec_resolution__accumulate_last_2_months_metric__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down
4 changes: 2 additions & 2 deletions ...test_filter_spec_resolution__derived_metric_with_different_parent_time_grains__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down Expand Up @@ -95,7 +95,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=YEAR,
),
Expand Down
2 changes: 1 addition & 1 deletion .../str/test_filter_spec_resolution__derived_metric_with_same_parent_time_grains__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down
4 changes: 2 additions & 2 deletions ...lookup.py/str/test_filter_spec_resolution__metrics_with_different_time_grains__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down Expand Up @@ -95,7 +95,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=YEAR,
),
Expand Down
2 changes: 1 addition & 1 deletion ...spec_lookup.py/str/test_filter_spec_resolution__metrics_with_same_time_grains__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down
2 changes: 1 addition & 1 deletion .../snapshots/test_spec_lookup.py/str/test_filter_spec_resolution__simple_metric__result.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ FilterSpecResolutionLookUp(
element_name='metric_time',
dimension_type=TIME,
properties=frozenset(
METRIC_TIME,
'METRIC_TIME',
),
time_granularity=MONTH,
),
Expand Down
2 changes: 1 addition & 1 deletion ...snapshots/test_dataflow_plan_builder.py/DataflowPlan/test_distinct_values_plan__dfp_0.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
<!-- element_name='country_latest', -->
<!-- dimension_type=CATEGORICAL, -->
<!-- entity_links=(EntityReference(element_name='listing'),), -->
<!-- properties=frozenset(LOCAL,), -->
<!-- properties=frozenset('LOCAL',), -->
<!-- ), -->
<!-- ), -->
<!-- ) -->
Expand Down
2 changes: 1 addition & 1 deletion ...test_dataflow_plan_builder.py/DataflowPlan/test_distinct_values_plan_with_join__dfp_0.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
<!-- element_name='country_latest', -->
<!-- dimension_type=CATEGORICAL, -->
<!-- entity_links=(EntityReference(element_name='listing'),), -->
<!-- properties=frozenset(LOCAL,), -->
<!-- properties=frozenset('LOCAL',), -->
<!-- ), -->
<!-- ), -->
<!-- ) -->
Expand Down
6 changes: 3 additions & 3 deletions ...est_dataflow_plan_builder.py/DataflowPlan/test_join_to_time_spine_with_filters__dfp_0.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<!-- ), -->
<!-- element_name='metric_time', -->
<!-- dimension_type=TIME, -->
<!-- properties=frozenset(METRIC_TIME,), -->
<!-- properties=frozenset('METRIC_TIME',), -->
<!-- time_granularity=DAY, -->
<!-- ), -->
<!-- ), -->
Expand All @@ -49,7 +49,7 @@
<!-- ), -->
<!-- element_name='metric_time', -->
<!-- dimension_type=TIME, -->
<!-- properties=frozenset(METRIC_TIME,), -->
<!-- properties=frozenset('METRIC_TIME',), -->
<!-- time_granularity=DAY, -->
<!-- ), -->
<!-- ), -->
Expand Down Expand Up @@ -91,7 +91,7 @@
<!-- ), -->
<!-- element_name='metric_time', -->
<!-- dimension_type=TIME, -->
<!-- properties=frozenset(METRIC_TIME,), -->
<!-- properties=frozenset('METRIC_TIME',), -->
<!-- time_granularity=DAY, -->
<!-- ), -->
<!-- ), -->
Expand Down
8 changes: 4 additions & 4 deletions ...pshots/test_dataflow_plan_builder.py/DataflowPlan/test_measure_constraint_plan__dfp_0.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
<!-- ), -->
<!-- ), -->
<!-- ), -->
<!-- properties=frozenset(JOINED,), -->
<!-- properties=frozenset('JOINED',), -->
<!-- ), -->
<!-- ), -->
<!-- ), -->
Expand Down Expand Up @@ -91,7 +91,7 @@
<!-- ), -->
<!-- ), -->
<!-- ), -->
<!-- properties=frozenset(JOINED,), -->
<!-- properties=frozenset('JOINED',), -->
<!-- ), -->
<!-- ), -->
<!-- ) -->
Expand Down Expand Up @@ -200,7 +200,7 @@
<!-- ), -->
<!-- ), -->
<!-- ), -->
<!-- properties=frozenset(JOINED,), -->
<!-- properties=frozenset('JOINED',), -->
<!-- ), -->
<!-- ), -->
<!-- ), -->
Expand Down Expand Up @@ -246,7 +246,7 @@
<!-- ), -->
<!-- ), -->
<!-- ), -->
<!-- properties=frozenset(JOINED,), -->
<!-- properties=frozenset('JOINED',), -->
<!-- ), -->
<!-- ), -->
<!-- ) -->
Expand Down
4 changes: 2 additions & 2 deletions ..._plan_builder.py/DataflowPlan/test_measure_constraint_with_reused_measure_plan__dfp_0.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<!-- entity_links=( -->
<!-- EntityReference(element_name='booking'), -->
<!-- ), -->
<!-- properties=frozenset(LOCAL,), -->
<!-- properties=frozenset('LOCAL',), -->
<!-- ), -->
<!-- ), -->
<!-- ), -->
Expand Down Expand Up @@ -72,7 +72,7 @@
<!-- element_name='is_instant', -->
<!-- dimension_type=CATEGORICAL, -->
<!-- entity_links=(EntityReference(element_name='booking'),), -->
<!-- properties=frozenset(LOCAL,), -->
<!-- properties=frozenset('LOCAL',), -->
<!-- ), -->
<!-- ), -->
<!-- ) -->
Expand Down
Loading

0 comments on commit 249a23f

Please sign in to comment.