-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve DAG Text Formatting #975
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks!
<!-- from_source = SqlSelectStatementNode(node_id=ss_10) --> | ||
<!-- where = None --> | ||
<!-- distinct = False --> | ||
<SqlSelectStatementNode> | ||
<!-- description = Pass Only Elements: ['visits',] --> | ||
<!-- node_id = ss_10 --> | ||
<!-- description = "Pass Only Elements: ['visits',]" --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we default to double quotes for these descriptions for better consistency? Not a strong opinion here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean how it uses "
in some cases but '
in others? It seems that's the behavior of repr()
/ pprint.pformat()
if it's a string or a string containing a quote.
ff71ec2
to
9b9bb09
Compare
bd04368
to
fc4fe7b
Compare
@@ -111,6 +112,10 @@ def accept_dag_node_visitor(self, visitor: DagNodeVisitor[VisitorOutputT]) -> Vi | |||
"""Visit this node.""" | |||
return visitor.visit_node(self) | |||
|
|||
def text_structure(self, formatter: MetricFlowDagTextFormatter = MetricFlowDagTextFormatter()) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change warms my cold grinchy heart....
fc4fe7b
to
89d5fe4
Compare
This adds a .text_structure() to the DAG classes for easier conversion to a text representation.
These are no longer necessary as the DAG classes now have a .text_structure().
This updates the logic for how max with is tracked / handled when recursively formatting a DAG to properly handle the indented / recursive case.
mf_pformat_many() makes the log look a little odd as it logs repr(str) instead of str.
89d5fe4
to
3d3d82a
Compare
Description
This PR:
Dag.text_structure() / DagNode.text_structure()
to make it easier to generate the text representation of a DAG.