Skip to content

Commit

Permalink
Fix bug in line-width control in DAG pretty printer (#1318)
Browse files Browse the repository at this point in the history
This PR fixes a bug with how the maximum width for pretty printing was
not correctly computed. Note that `SqlStringExpression(node_id=str_0
sql_expr='foo')` is longer than expected - this is due to a `__repr__`
issue that will be addressed in a follow up.
  • Loading branch information
plypaul authored Jul 11, 2024
1 parent ea1748f commit 2a5985d
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 13 deletions.
4 changes: 2 additions & 2 deletions metricflow-semantics/metricflow_semantics/dag/dag_to_text.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def __init__(self, max_width: int) -> None: # noqa: D107
def update_max_width_for_indented_section(self, indent_prefix: str) -> Iterator[None]:
"""Context manager used to wrap the code that prints an indented section."""
previous_max_width = self._current_max_width
self._current_max_width = max(0, self._current_max_width - len(indent_prefix))
self._current_max_width = max(1, self._current_max_width - len(indent_prefix))
yield None
self._current_max_width = previous_max_width

Expand Down Expand Up @@ -101,7 +101,7 @@ def _format_to_text(self, node: DagNode, inner_contents: Optional[str]) -> str:
displayed_property.value,
# The string representation of displayed_property.value will be wrapped with "<!-- ", " -->" so subtract
# the width of those.
max_line_length=max_width - len("<!-- ") - len(" -->"),
max_line_length=max(1, max_width - len("<!-- ") - len(" -->")),
indent_prefix=self._value_indent_prefix,
)

Expand Down
31 changes: 20 additions & 11 deletions tests_metricflow/mf_logging/test_dag_to_text.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,24 +70,33 @@ def _run_mf_pformat() -> None:
<SqlQueryPlan>
<SqlSelectStatementNode>
<!-- description = -->
<!-- test -->
<!-- node_id = -->
<!-- ss_0 -->
<!-- col0 = -->
<!-- SqlSelectColumn(expr=SqlStringExpression(node_id=str_0 sql_expr='foo'), column_alias='bar') -->
<!-- 'test' -->
<!-- node_id = -->
<!-- NodeId( -->
<!-- id_str='ss_0', -->
<!-- ) -->
<!-- col0 = -->
<!-- SqlSelectColumn( -->
<!-- expr=SqlStringExpression(node_id=str_0 sql_expr='foo'), -->
<!-- column_alias='bar', -->
<!-- ) -->
<!-- from_source = -->
<!-- SqlTableFromClauseNode(node_id=tfc_0) -->
<!-- where = -->
<!-- None -->
<!-- distinct = -->
<!-- False -->
<SqlTableFromClauseNode>
<!-- description = -->
<!-- Read from schema.table -->
<!-- node_id = -->
<!-- tfc_0 -->
<!-- table_id = -->
<!-- schema.table -->
<!-- description = -->
<!-- ('Read ' -->
<!-- 'from ' -->
<!-- 'schema.table') -->
<!-- node_id = -->
<!-- NodeId( -->
<!-- id_str='tfc_0', -->
<!-- ) -->
<!-- table_id = -->
<!-- 'schema.table' -->
</SqlTableFromClauseNode>
</SqlSelectStatementNode>
</SqlQueryPlan>
Expand Down

0 comments on commit 2a5985d

Please sign in to comment.