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

Add path viz & lineage example #174

Merged
merged 9 commits into from
May 25, 2023
Merged

Add path viz & lineage example #174

merged 9 commits into from
May 25, 2023

Conversation

skrawcz
Copy link
Collaborator

@skrawcz skrawcz commented May 22, 2023

Adds path visualization and refactors the internal graphviz function a little.

Changes

  • driver
  • graph and test
  • examples & docs

How I tested this

  • locally
  • unit tests for existing code

Notes

  • see image files for example

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.

Splitting into user nodes is now obsolete with the node_modifiers
dictionary. So this refactors the code internally to remove the `user_nodes`
argument, and instead pass in all nodes with the appropriate node modifiers.

I seemed to also fix a bug that's been there in the dot file metadata -- in that
the test was adding two nodes for the same function.
This is useful for debugging and zooming in on a particular path.
People who want to get more out of lineage will want to use this
function to help them document, debug, and understand how
particular things are related.
This does not return the actual order of nodes that represents the path, because that
seems more trouble than it's worth. Instead it'll return the list of nodes that constitute the path.
This should suffice for someone to inspect tags.
Didn't sound right without the `the`.
Copy link
Collaborator

@elijahbenizzy elijahbenizzy left a comment

Choose a reason for hiding this comment

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

LGTM -- some small nits

hamilton/driver.py Show resolved Hide resolved
hamilton/driver.py Outdated Show resolved Hide resolved
hamilton/graph.py Show resolved Hide resolved
hamilton/graph.py Show resolved Hide resolved
hamilton/graph.py Outdated Show resolved Hide resolved
hamilton/graph.py Outdated Show resolved Hide resolved
We have a `display_downstream_of` function, so
there should be an analogous `display_upstream_of` function.

It behaves the same. Tested locally.
@skrawcz skrawcz changed the title Add path viz Add path viz & lineage example May 24, 2023
@skrawcz skrawcz marked this pull request as ready for review May 25, 2023 06:45
This adds documentation and code examples on how
to utilize Hamilton's lineage capabilities.

There will be a couple follow up commits to change
the example to so that people can go back in time to this
current state and compare how things have changed.
This shows some changes to the Titanic DAG.
This is what people will play with. Once everything
is merged we'll update the README so that
people can go back to the prior commit and
run the same code.
We're doing this for consistency, since this complements `get_upstream_nodes`.

This is fine to change, since the `graph.py` module is seen an internal API
and not a publicly facing one.  But just in case someone is somehow using it, I'm leaving it in, and
delegating to the new function with a warning message.
This is to house the possible visual modifiers for a node.
This is an enum to (1) keep them grouped, (2) they were
being used in a boolean fashion, so the presence of the enum
is enough.
Copy link
Collaborator

@elijahbenizzy elijahbenizzy left a comment

Choose a reason for hiding this comment

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

🚢 🇮🇹

@elijahbenizzy elijahbenizzy merged commit 0122dce into main May 25, 2023
@elijahbenizzy elijahbenizzy deleted the add_path_viz branch May 25, 2023 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants