Skip to content

Conversation

@Exudev
Copy link
Contributor

@Exudev Exudev commented Jan 17, 2026

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
Copy link
Contributor Author

Exudev commented Jan 19, 2026

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

Copy link
Contributor

@skrawcz skrawcz left a comment

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
Copy link
Contributor

skrawcz commented Jan 19, 2026

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
Copy link
Contributor Author

Exudev commented Jan 20, 2026

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

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

@Exudev
Copy link
Contributor Author

Exudev commented Jan 20, 2026

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
Copy link
Contributor

skrawcz commented Jan 20, 2026

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
Copy link
Contributor

skrawcz commented Jan 24, 2026

@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
Copy link
Contributor Author

Exudev commented Jan 24, 2026

@skrawcz done :D

Copy link
Contributor

@skrawcz skrawcz left a comment

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
Copy link
Contributor Author

Exudev commented Jan 24, 2026

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

@skrawcz
Copy link
Contributor

skrawcz commented Jan 24, 2026

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