Fix off-by-one issue in pretty-formatter (#1665)

The pretty-format methods have a `max_line_length` that is used to keep
the formatted output within the given number of columns when possible.
The argument is supposed to be >0, but in some cases, `max(0, ...)` was
used, resulting in errors when the limit was hit. This PR updates those
calls to `max(1, ...)`.

This PR includes some commits to better organize the tests - please view
by commit.

Author: plypaul

metricflow-semantics/metricflow_semantics/mf_logging/pretty_print.py

@@ -93,7 +93,7 @@ def _handle_sequence_obj(
         # Convert each item to a pretty string.
         items_as_str = tuple(
             self._handle_any_obj(
-                list_item, remaining_line_width=max(0, remaining_line_width - len(self._indent_prefix))
+                list_item, remaining_line_width=max(1, remaining_line_width - len(self._indent_prefix))
             )
             for list_item in list_like_obj
         )
@@ -216,7 +216,7 @@ def _handle_indented_key_value_item(  # type: ignore[misc]
         # ]
 
         # See if the value fits in the previous line.
-        remaining_width_for_value = max(0, remaining_line_width - len(result_lines[-1]))
+        remaining_width_for_value = max(1, remaining_line_width - len(result_lines[-1]))
         value_str = self._handle_any_obj(value, remaining_line_width=remaining_width_for_value)
         value_lines = value_str.splitlines()
 
@@ -456,7 +456,7 @@ def mf_pformat_dict(  # type: ignore
         else:
             value_str = mf_pformat(
                 obj=value,
-                max_line_length=max(0, max_line_length - len(indent_prefix)),
+                max_line_length=max(1, max_line_length - len(indent_prefix)),
                 indent_prefix=indent_prefix,
                 include_object_field_names=include_object_field_names,
                 include_none_object_fields=include_none_object_fields,

metricflow-semantics/tests_metricflow_semantics/mf_logging/__init__.py

@@ -4,9 +4,10 @@
 
 from metricflow_semantics.formatting.formatting_helpers import mf_dedent
 from metricflow_semantics.mf_logging.lazy_formattable import LazyFormat
-from tests_metricflow_semantics.mf_logging.recorded_logging_context import RecordingLogHandler, recorded_logging_context
 from typing_extensions import override
 
+from tests_metricflow_semantics.mf_logging.recorded_logging_context import RecordingLogHandler, recorded_logging_context
+
 logger = logging.getLogger(__name__)
 
 

metricflow-semantics/tests_metricflow_semantics/mf_logging/test_lazy_format.py

@@ -0,0 +1,94 @@
+from __future__ import annotations
+
+import logging
+import textwrap
+
+from metricflow_semantics.formatting.formatting_helpers import mf_dedent
+from metricflow_semantics.mf_logging.pretty_print import mf_pformat_dict
+
+logger = logging.getLogger(__name__)
+
+
+def test_pformat_many() -> None:  # noqa: D103
+    result = mf_pformat_dict("Example description:", obj_dict={"object_0": (1, 2, 3), "object_1": {4: 5}})
+
+    assert (
+        textwrap.dedent(
+            """\
+            Example description:
+              object_0: (1, 2, 3)
+              object_1: {4: 5}
+            """
+        ).rstrip()
+        == result
+    )
+
+
+def test_pformat_many_with_raw_strings() -> None:  # noqa: D103
+    result = mf_pformat_dict("Example description:", obj_dict={"object_0": "foo\nbar"}, preserve_raw_strings=True)
+
+    assert (
+        textwrap.dedent(
+            """\
+            Example description:
+              object_0:
+                foo
+                bar
+            """
+        ).rstrip()
+        == result
+    )
+
+
+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
+    )
+
+
+def test_pformat_dict_with_pad_sections_with_newline() -> None:
+    """Test `mf_pformat_dict` with new lines between sections."""
+    result = mf_pformat_dict(obj_dict={"object_0": (1, 2, 3), "object_1": {4: 5}}, pad_items_with_newlines=True)
+
+    assert (
+        mf_dedent(
+            """
+            object_0: (1, 2, 3)
+
+            object_1: {4: 5}
+            """
+        )
+        == result
+    )
+
+
+def test_pformat_many_with_strings() -> None:  # noqa: D103
+    result = mf_pformat_dict("Example description:", obj_dict={"object_0": "foo\nbar"})
+    assert (
+        textwrap.dedent(
+            """\
+            Example description:
+              object_0: 'foo\\nbar'
+            """
+        ).rstrip()
+        == result
+    )
+
+
+def test_minimal_length() -> None:
+    """Test where the max_line_length is the minimal length."""
+    assert mf_pformat_dict("Example output", {"foo": "bar"}, max_line_length=1) == mf_dedent(
+        """
+        Example output
+          foo: 'bar'
+        """
+    )

metricflow-semantics/tests_metricflow_semantics/mf_logging/test_pformat_dict.py

@@ -7,10 +7,9 @@
 
 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_dict
+from metricflow_semantics.mf_logging.pretty_print import mf_pformat
 from metricflow_semantics.test_helpers.metric_time_dimension import MTD_SPEC_DAY
 from typing_extensions import override
 
@@ -146,50 +145,6 @@ def test_pydantic_model() -> None:  # noqa: D103
     )
 
 
-def test_pformat_many() -> None:  # noqa: D103
-    result = mf_pformat_dict("Example description:", obj_dict={"object_0": (1, 2, 3), "object_1": {4: 5}})
-
-    assert (
-        textwrap.dedent(
-            """\
-            Example description:
-              object_0: (1, 2, 3)
-              object_1: {4: 5}
-            """
-        ).rstrip()
-        == result
-    )
-
-
-def test_pformat_many_with_raw_strings() -> None:  # noqa: D103
-    result = mf_pformat_dict("Example description:", obj_dict={"object_0": "foo\nbar"}, preserve_raw_strings=True)
-
-    assert (
-        textwrap.dedent(
-            """\
-            Example description:
-              object_0:
-                foo
-                bar
-            """
-        ).rstrip()
-        == result
-    )
-
-
-def test_pformat_many_with_strings() -> None:  # noqa: D103
-    result = mf_pformat_dict("Example description:", obj_dict={"object_0": "foo\nbar"})
-    assert (
-        textwrap.dedent(
-            """\
-            Example description:
-              object_0: 'foo\\nbar'
-            """
-        ).rstrip()
-        == result
-    )
-
-
 def test_custom_pretty_print() -> None:
     """Check that `MetricFlowPrettyFormattable` can be used to override the result when using MF's pretty-printer."""
 
@@ -204,34 +159,3 @@ 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
-    )
-
-
-def test_pformat_dict_with_pad_sections_with_newline() -> None:
-    """Test `mf_pformat_dict` with new lines between sections."""
-    result = mf_pformat_dict(obj_dict={"object_0": (1, 2, 3), "object_1": {4: 5}}, pad_items_with_newlines=True)
-
-    assert (
-        mf_dedent(
-            """
-            object_0: (1, 2, 3)
-
-            object_1: {4: 5}
-            """
-        )
-        == result
-    )