Repository navigation
Conversation
Nodes built from @config.when or @config.when_not functions were labelled with the node name plus the config keys, so their display_name tag was ignored. The tag lookup is now a shared helper, and the config label is built from the resolved display name with the config suffix kept. A test covers both decorator stacking orders. Closes apache#1761
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #1761
Nodes built from
@config.whenor@config.when_notfunctions ignored theirdisplay_nametag in visualizations.create_graphviz_graphrebuilt their label asf"{n.name}: {config_key}"and passed it as the explicitnameto_get_node_label, which gives that argument priority over the tag, so these nodes always showed asa: modeinstead ofPretty A: mode.Changes
hamilton/graph.py: moved thedisplay_nametag lookup (including the existing "list means first element" handling) out of_get_node_labelinto a small_get_display_namehelper insidecreate_graphviz_graph._get_node_labeluses it when no explicit name is given, as before.f"{_get_display_name(n)}: {config_key}", so the tag is used and the config suffix is kept. Nodes without the tag still get<node name>: <config keys>.How I tested this
test_create_graphviz_graph_display_name_on_config_nodesintests/test_graph.py. It builds a graph from a@config.whenfunction with@tag(display_name=...)on the outside and a@config.when_notfunction with the tag on the inside, and asserts that the Digraph source contains<b>Pretty A: mode</b>and<b>Pretty C: mode</b>and no longer contains<b>a: mode</b>or<b>c: mode</b>. It fails on main and passes with this change.tests/test_graph.py,tests/function_modifiersandtests/plugins/test_polars*.pybefore and after the change. The only failures are the same on both sides and come from my local environment (no Graphvizdotbinary for the render tests, polars 2.x and a missingxlsxwriterfor the polars plugin tests).display_all_functions()andvisualize_execution()now both showPretty A: mode,Pretty C: modeandPretty D.ruff checkandruff format --checkpass on the changed files.Notes
No change to the explicit
nameoverride in_get_node_label; it still takes priority over the tag, so materializer and other callers behave as before.Checklist