Skip to content

Enhance visualization with display_name tag support#1445

Merged
skrawcz merged 7 commits into
apache:mainfrom
Exudev:main
Jan 24, 2026
Merged

Enhance visualization with display_name tag support#1445
skrawcz merged 7 commits into
apache:mainfrom
Exudev:main

Conversation

@Exudev

@Exudev Exudev commented Jan 17, 2026

Copy link
Copy Markdown
Contributor

This update introduces the tag for nodes in graph visualizations, allowing for human-readable labels while maintaining valid Python identifiers as function names. The changes include:

  • Documentation updates to explain the usage of in visualizations.
  • Modifications to the graph creation logic to utilize when available.
  • New tests to ensure is correctly applied in visualizations and that HTML characters are properly escaped.
  • Example functions demonstrating the use of in a new resource file.

Issue

This update introduces the  tag for nodes in graph visualizations, allowing for human-readable labels while maintaining valid Python identifiers as function names. The changes include:

- Documentation updates to explain the usage of  in visualizations.
- Modifications to the graph creation logic to utilize  when available.
- New tests to ensure  is correctly applied in visualizations and that HTML characters are properly escaped.
- Example functions demonstrating the use of  in a new resource file.

This feature improves the readability of visualizations for stakeholders while keeping the codebase Pythonic.
…special characters are properly escaped in the label.
@Exudev Exudev marked this pull request as ready for review January 17, 2026 16:13
@Exudev Exudev marked this pull request as draft January 17, 2026 16:20
This update improves the handling of the display_name tag in graph visualizations by adding support for cases where display_name is a list. The first element of the list will now be used as the display name. Additionally, new tests have been added to verify this functionality, ensuring that the correct display names are rendered in the graph output. This enhancement contributes to better readability and usability of visualizations.
@Exudev Exudev marked this pull request as ready for review January 17, 2026 16:40
@Exudev Exudev mentioned this pull request Jan 17, 2026
@Exudev

Exudev commented Jan 19, 2026

Copy link
Copy Markdown
Contributor Author

Greetings, @skrawcz I was attempting to assign you as a reviewer, but I believe I don't have the appropriate permissions.

Comment thread tests/test_graph.py Outdated

@skrawcz skrawcz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking good -- will let CI run and see if things pass -- otherwise one small nit with respect to imports in the test, if you could just import it at the top level please.

@skrawcz

skrawcz commented Jan 19, 2026

Copy link
Copy Markdown
Contributor

The unit test errors are unrelated -- mind showing evidence yours passes?

Unit test errors are:

  1. related to protobuff and mlflow. Likely need to pin protobuff for mlflow extension / optional install.
  2. related to pandas changing an API. Not sure when this started failing, but fix is likely looking at the pandas version and deciding which API to use...

@Exudev

Exudev commented Jan 20, 2026

Copy link
Copy Markdown
Contributor Author

The unit test errors are unrelated -- mind showing evidence yours passes?

Screenshot 2026-01-19 at 8 18 54 PM @skrawcz

@Exudev

Exudev commented Jan 20, 2026

Copy link
Copy Markdown
Contributor Author

Happy to help with other issues once this ticket is closed! If there are any documented open issues, I'd be glad to take them on.

@skrawcz

skrawcz commented Jan 20, 2026

Copy link
Copy Markdown
Contributor

One more thing - the sphinx build broke with:

/home/runner/work/hamilton/hamilton/docs/concepts/visualization.rst:170: WARNING: Title underline too short.

@Exudev Exudev requested a review from skrawcz January 20, 2026 00:27
@skrawcz

skrawcz commented Jan 24, 2026

Copy link
Copy Markdown
Contributor

@Exudev almost there -- seems like there is a linting issue :/ mind installing the pre-commit hooks and running them? It should fix it since it was ruff that complained.

@Exudev

Exudev commented Jan 24, 2026

Copy link
Copy Markdown
Contributor Author

@skrawcz done :D

@skrawcz skrawcz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM thanks!

We can fix the unrelated unit tests in another PR...

@skrawcz skrawcz merged commit d570d3f into apache:main Jan 24, 2026
0 of 5 checks passed
@Exudev

Exudev commented Jan 24, 2026

Copy link
Copy Markdown
Contributor Author

Of course anything, just assign me the issues and ill be on it.

@skrawcz

skrawcz commented Jan 24, 2026

Copy link
Copy Markdown
Contributor

Created #1450 if you want to comment on it @Exudev

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