Skip to content
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

feat(graphviz): node- and edge-specific custom attributes #8527

Merged

Conversation

peter-gy
Copy link
Contributor

@peter-gy peter-gy commented Mar 3, 2024

Description of changes

Follow-up to #8510.

Adds support to optionally specifying custom Graphviz node attributes and edge attributes based on arbitrary logic described by user-defined callbacks: node_attr_callback and edge_attr_callback.

With this, more complex visualization needs can be addressed, such as:

  • I want to use a custom shape and fill color for all the Field nodes
  • I want to have custom edge style and color between Field and Comparison nodes

Consider the example below.

import ibis
import ibis.expr.operations as ops

left = ibis.table(dict(a="int64", b="string"), name="left")
right = ibis.table(dict(b="string", c="int64", d="string"), name="right")
expr = (
    left.inner_join(right, "b")
    .select(left.a, b=right.c, c=right.d)
    .mutate(
        arrays=ibis.array([1, 2, 3]),
    )
)

def get_node_attributes(node):
    if isinstance(node, ops.Literal):
        return {"shape": "hexagon", "style": "filled", "fillcolor": "yellow"}

def get_edge_attributes(u, v):
    if isinstance(u, ops.Field) and isinstance(v, ops.Comparison):
        return {"style": "dashed", "color": "red"}

    if isinstance(v, ops.Field):
        return {"arrowhead": "box", "color": "blue"}


expr.visualize(
    label_edges=True,
    node_attr={"color": "green", "fontname": "Roboto Mono"},
    node_attr_callback=get_node_attributes,
    edge_attr={"fontsize": "12", "fontname": "Comic Sans MS"},
    edge_attr_callback=get_edge_attributes,
)

custom-attrs

@peter-gy
Copy link
Contributor Author

peter-gy commented Mar 3, 2024

An API design consideration we have here is whether we want to merge node_attr and node_attr_callback as well as edge_attr and edge_attr_callback to be of type dict[str,str] | NodeAttributeCallback | None and dict[str,str] | EdgeAttributeCallback | None respectively so that we don’t impose additional cognitive load on users with parameter variations.

What do you think?

@cpcloud
Copy link
Member

cpcloud commented Mar 3, 2024

Pretty fancy graph there!

Keeping the separation between "default attributes for every node" and "a function that can be used to override specific nodes" seems like a good separation to me.

While I don't love the _callback naming, I'm not sure there's something that significantly better.

@peter-gy
Copy link
Contributor Author

peter-gy commented Mar 3, 2024

Pretty fancy graph there!

Keeping the separation between "default attributes for every node" and "a function that can be used to override specific nodes" seems like a good separation to me.

While I don't love the _callback naming, I'm not sure there's something that significantly better.

I agree with the need for the separation.

What about _getter as a suffix?

@cpcloud
Copy link
Member

cpcloud commented Mar 3, 2024

getter SGTM!

@cpcloud cpcloud added this to the 9.0 milestone Mar 5, 2024
Copy link
Member

@cpcloud cpcloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Niiiiice!

@cpcloud cpcloud added ux User experience related issues graphviz Issues or PRs related to visualizing ibis expresssions using graphviz labels Mar 5, 2024
@cpcloud cpcloud merged commit 98c52aa into ibis-project:main Mar 5, 2024
78 checks passed
@peter-gy peter-gy deleted the feat/custom-graph-attributes-per-node branch March 5, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
graphviz Issues or PRs related to visualizing ibis expresssions using graphviz ux User experience related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants