Assorted formatting / logging updates (#1518)

This PR contains an assortment of updates related to logging /
formatting. Although not really related, since they were small, they
were put into a single PR. Please view by commit.

metricflow-semantics/metricflow_semantics/mf_logging/lazy_formattable.py

@@ -5,7 +5,7 @@
 
 from typing_extensions import override
 
-from metricflow_semantics.mf_logging.pretty_print import mf_pformat_many
+from metricflow_semantics.mf_logging.pretty_print import mf_pformat_dict
 
 
 class LazyFormat:
@@ -52,7 +52,7 @@ def _str_value(self) -> str:
             message = self._message()
         else:
             message = self._message
-        return mf_pformat_many(message, self._kwargs, preserve_raw_strings=True)
+        return mf_pformat_dict(message, self._kwargs, preserve_raw_strings=True)
 
     @override
     def __str__(self) -> str:

metricflow-semantics/metricflow_semantics/mf_logging/pretty_print.py

@@ -5,7 +5,7 @@
 from collections.abc import Mapping
 from dataclasses import fields, is_dataclass
 from enum import Enum
-from typing import Any, Dict, List, Optional, Sized, Tuple, Union
+from typing import Any, List, Optional, Sized, Tuple, Union
 
 from dsi_pydantic_shim import BaseModel
 
@@ -428,9 +428,9 @@ def mf_pformat(  # type: ignore
         return str(obj)
 
 
-def mf_pformat_many(  # type: ignore
-    description: str,
-    obj_dict: Dict[str, Any],
+def mf_pformat_dict(  # type: ignore
+    description: Optional[str] = None,
+    obj_dict: Optional[Mapping[str, Any]] = None,
     max_line_length: int = 120,
     indent_prefix: str = "  ",
     include_object_field_names: bool = True,
@@ -444,7 +444,8 @@ def mf_pformat_many(  # type: ignore
     representation of the string. e.g. if value="foo", then "foo" instead of "'foo'". Useful for values that contain
     newlines.
     """
-    lines: List[str] = [description]
+    lines: List[str] = [description] if description is not None else []
+    obj_dict = obj_dict or {}
     for key, value in obj_dict.items():
         if preserve_raw_strings and isinstance(value, str):
             value_str = value
@@ -471,5 +472,9 @@ def mf_pformat_many(  # type: ignore
         else:
             item_block_lines = (f"{key}: {value_str}",)
         item_block = "\n".join(item_block_lines)
-        lines.append(indent(item_block))
+
+        if description is None:
+            lines.append(item_block)
+        else:
+            lines.append(indent(item_block))
     return "\n".join(lines)

metricflow-semantics/metricflow_semantics/query/group_by_item/candidate_push_down/push_down_visitor.py

@@ -14,7 +14,7 @@
 
 from metricflow_semantics.mf_logging.formatting import indent
 from metricflow_semantics.mf_logging.lazy_formattable import LazyFormat
-from metricflow_semantics.mf_logging.pretty_print import mf_pformat, mf_pformat_many
+from metricflow_semantics.mf_logging.pretty_print import mf_pformat, mf_pformat_dict
 from metricflow_semantics.model.semantic_manifest_lookup import SemanticManifestLookup
 from metricflow_semantics.model.semantics.element_filter import LinkableElementFilter
 from metricflow_semantics.query.group_by_item.candidate_push_down.group_by_item_candidate import GroupByItemCandidateSet
@@ -82,7 +82,7 @@ def __post_init__(self) -> None:  # noqa: D105
         # If there are errors, there shouldn't be any candidate sets.
         assert (not self.issue_set.has_errors and not self.candidate_set.is_empty) or (
             self.issue_set.has_errors and self.candidate_set.is_empty
-        ), mf_pformat_many(
+        ), mf_pformat_dict(
             "candidate_set / issue_set mismatch:", {"candidate_set": self.candidate_set, "issue_set": self.issue_set}
         )
 

metricflow-semantics/metricflow_semantics/query/query_resolver.py

@@ -8,7 +8,7 @@
 from dbt_semantic_interfaces.references import MeasureReference, MetricReference, SemanticModelReference
 
 from metricflow_semantics.mf_logging.lazy_formattable import LazyFormat
-from metricflow_semantics.mf_logging.pretty_print import mf_pformat, mf_pformat_many
+from metricflow_semantics.mf_logging.pretty_print import mf_pformat, mf_pformat_dict
 from metricflow_semantics.mf_logging.runtime import log_runtime
 from metricflow_semantics.model.semantic_manifest_lookup import SemanticManifestLookup
 from metricflow_semantics.model.semantic_model_derivation import SemanticModelDerivation
@@ -557,7 +557,7 @@ def _resolve_query(self, resolver_input_for_query: ResolverInputForQuery) -> Met
         if len(models_not_in_manifest) > 0:
             logger.error(
                 LazyFormat(
-                    lambda: mf_pformat_many(
+                    lambda: mf_pformat_dict(
                         "Semantic references that aren't in the manifest were found in the set used in "
                         "a query. This is a bug, and to avoid potential issues, they will be filtered out.",
                         {"models_not_in_manifest": models_not_in_manifest},

metricflow-semantics/metricflow_semantics/test_helpers/snapshot_helpers.py

@@ -56,6 +56,7 @@ def assert_snapshot_text_equal(
     expectation_description: Optional[str] = None,
 ) -> None:
     """Similar to assert_plan_snapshot_text_equal(), but with more controls on how the snapshot paths are generated."""
+    logger.debug(LazyFormat(lambda: "Generated snapshot text:\n" + indent(snapshot_text)))
     file_path = (
         str(
             snapshot_path_prefix(

metricflow-semantics/tests_metricflow_semantics/collection_helpers/test_pretty_print.py

@@ -7,9 +7,10 @@
 
 from dbt_semantic_interfaces.implementations.elements.dimension import PydanticDimension
 from dbt_semantic_interfaces.type_enums import DimensionType
+from metricflow_semantics.formatting.formatting_helpers import mf_dedent
 from metricflow_semantics.mf_logging.formatting import indent
 from metricflow_semantics.mf_logging.pretty_formattable import MetricFlowPrettyFormattable
-from metricflow_semantics.mf_logging.pretty_print import mf_pformat, mf_pformat_many
+from metricflow_semantics.mf_logging.pretty_print import mf_pformat, mf_pformat_dict
 from metricflow_semantics.test_helpers.metric_time_dimension import MTD_SPEC_DAY
 from typing_extensions import override
 
@@ -145,7 +146,7 @@ def test_pydantic_model() -> None:  # noqa: D103
 
 
 def test_pformat_many() -> None:  # noqa: D103
-    result = mf_pformat_many("Example description:", obj_dict={"object_0": (1, 2, 3), "object_1": {4: 5}})
+    result = mf_pformat_dict("Example description:", obj_dict={"object_0": (1, 2, 3), "object_1": {4: 5}})
 
     assert (
         textwrap.dedent(
@@ -160,7 +161,7 @@ def test_pformat_many() -> None:  # noqa: D103
 
 
 def test_pformat_many_with_raw_strings() -> None:  # noqa: D103
-    result = mf_pformat_many("Example description:", obj_dict={"object_0": "foo\nbar"}, preserve_raw_strings=True)
+    result = mf_pformat_dict("Example description:", obj_dict={"object_0": "foo\nbar"}, preserve_raw_strings=True)
 
     assert (
         textwrap.dedent(
@@ -176,7 +177,7 @@ def test_pformat_many_with_raw_strings() -> None:  # noqa: D103
 
 
 def test_pformat_many_with_strings() -> None:  # noqa: D103
-    result = mf_pformat_many("Example description:", obj_dict={"object_0": "foo\nbar"})
+    result = mf_pformat_dict("Example description:", obj_dict={"object_0": "foo\nbar"})
     assert (
         textwrap.dedent(
             """\
@@ -202,3 +203,18 @@ def pretty_format(self) -> Optional[str]:
             return f"{self.__class__.__name__}({self.field_0:.2f})"
 
     assert mf_pformat(_ExampleDataclass(1.2345)) == f"{_ExampleDataclass.__name__}(1.23)"
+
+
+def test_pformat_dict_with_empty_message() -> None:
+    """Test `mf_pformat_dict` without a description."""
+    result = mf_pformat_dict(obj_dict={"object_0": (1, 2, 3), "object_1": {4: 5}})
+
+    assert (
+        mf_dedent(
+            """
+            object_0: (1, 2, 3)
+            object_1: {4: 5}
+            """
+        )
+        == result
+    )

metricflow/data_table/mf_table.py

@@ -9,7 +9,7 @@
 
 import tabulate
 from metricflow_semantics.mf_logging.formatting import indent
-from metricflow_semantics.mf_logging.pretty_print import mf_pformat, mf_pformat_many
+from metricflow_semantics.mf_logging.pretty_print import mf_pformat, mf_pformat_dict
 from typing_extensions import Self
 
 from metricflow.data_table.column_types import CellValue, InputCellValue, row_cell_types
@@ -47,7 +47,7 @@ def __post_init__(self) -> None:  # noqa: D105
             # Check that the type of the object in the rows match.
             for column_index, cell_value in enumerate(row):
                 expected_cell_value_type = self.column_descriptions[column_index].column_type
-                assert cell_value is None or isinstance(cell_value, expected_cell_value_type), mf_pformat_many(
+                assert cell_value is None or isinstance(cell_value, expected_cell_value_type), mf_pformat_dict(
                     "Cell value type mismatch.",
                     {
                         "row_index": row_index,
@@ -59,7 +59,7 @@ def __post_init__(self) -> None:  # noqa: D105
                 )
                 # Check that datetimes don't have a timezone set.
                 if isinstance(cell_value, datetime.datetime):
-                    assert cell_value.tzinfo is None, mf_pformat_many(
+                    assert cell_value.tzinfo is None, mf_pformat_dict(
                         "Time zone provided for datetime.",
                         {
                             "row_index": row_index,

metricflow/dataset/sql_dataset.py

@@ -5,6 +5,7 @@
 from dbt_semantic_interfaces.references import SemanticModelReference
 from metricflow_semantics.assert_one_arg import assert_exactly_one_arg_set
 from metricflow_semantics.instances import EntityInstance, InstanceSet
+from metricflow_semantics.mf_logging.lazy_formattable import LazyFormat
 from metricflow_semantics.specs.column_assoc import ColumnAssociation
 from metricflow_semantics.specs.dimension_spec import DimensionSpec
 from metricflow_semantics.specs.entity_spec import EntitySpec
@@ -42,7 +43,9 @@ def __init__(
     def sql_node(self) -> SqlQueryPlanNode:  # noqa: D102
         node_to_return = self._sql_select_node or self._sql_node
         if node_to_return is None:
-            raise RuntimeError("This node was not created with a SQL node.")
+            raise RuntimeError(
+                "This node was not created with a SQL node. This should have been prevented by the initializer."
+            )
         return node_to_return
 
     @property
@@ -52,7 +55,14 @@ def checked_sql_select_node(self) -> SqlSelectStatementNode:
         Otherwise, an exception is thrown.
         """
         if self._sql_select_node is None:
-            raise RuntimeError(f"{self} was created with a SQL node that is not a {SqlSelectStatementNode}")
+            raise RuntimeError(
+                str(
+                    LazyFormat(
+                        f"{self.__class__.__name__} was created with a SQL node that is not a {SqlSelectStatementNode.__name__}",
+                        sql_node=self.sql_node.structure_text(),
+                    )
+                )
+            )
         return self._sql_select_node
 
     def column_associations_for_entity(

tests_metricflow/sql/compare_data_table.py

@@ -6,7 +6,7 @@
 from dataclasses import dataclass
 from typing import Dict, Optional, SupportsFloat
 
-from metricflow_semantics.mf_logging.pretty_print import mf_pformat_many
+from metricflow_semantics.mf_logging.pretty_print import mf_pformat_dict
 
 from metricflow.data_table.column_types import CellValue
 from metricflow.data_table.mf_table import MetricFlowDataTable
@@ -106,7 +106,7 @@ def check_data_tables_are_equal(
 
     if expected_table.column_names != actual_table.column_names:
         raise ValueError(
-            mf_pformat_many(
+            mf_pformat_dict(
                 "Column descriptions do not match.",
                 {
                     "expected_table_column_names": expected_table.column_names,
@@ -117,7 +117,7 @@ def check_data_tables_are_equal(
 
     if expected_table.row_count != actual_table.row_count:
         raise ValueError(
-            mf_pformat_many(
+            mf_pformat_dict(
                 "Row counts do not match.",
                 dict(
                     **{
@@ -132,7 +132,7 @@ def check_data_tables_are_equal(
 
     if not allow_empty and expected_table.row_count == 0:
         raise ValueError(
-            mf_pformat_many(
+            mf_pformat_dict(
                 f"Expected table is empty and {allow_empty=}. This may indicate an error in configuring the test.",
                 _generate_table_diff_fields(expected_table=expected_table, actual_table=actual_table),
                 preserve_raw_strings=True,
@@ -143,7 +143,7 @@ def check_data_tables_are_equal(
 
     if mismatch is not None:
         raise ValueError(
-            mf_pformat_many(
+            mf_pformat_dict(
                 mismatch.message,
                 dict(
                     **{

tests_metricflow/sql/compare_sql_plan.py

@@ -36,6 +36,9 @@ def assert_default_rendered_sql_equal(
         plan=sql_query_plan,
         plan_snapshot_text=rendered_sql,
         plan_snapshot_file_extension=".sql",
+        incomparable_strings_replacement_function=make_schema_replacement_function(
+            system_schema=mf_test_configuration.mf_system_schema, source_schema=mf_test_configuration.mf_source_schema
+        ),
     )