-
Couldn't load subscription status.
- Fork 31
feat(graph): add --reduce option to to-dot command #758
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
base: main
Are you sure you want to change the base?
Conversation
Add a new --reduce flag to the graph to-dot command that filters the dependency graph to only include nodes with fromager customizations (overrides, settings, patches, plugins, or pre-built wheels). The reduced graph preserves dependency relationships by creating direct edges between customized nodes, skipping intermediate nodes without customizations. Original requirement specifications are tracked and displayed in edge labels for better clarity. Key changes: - Add --reduce command line option to to-dot command - Implement has_customizations() to identify nodes with customizations - Implement reduce_graph() to build filtered dependency relationships - Track original requirement strings for proper edge labeling - Use req_type.is_build_requirement property for build dependency checks This feature helps visualize which packages in a dependency tree actually have fromager customizations while maintaining essential dependency relationships. Cursor chat transcript: https://gist.github.com/dhellmann/e362954e00a7270dcc71037382f53853 Co-authored-by: Claude 3.5 Sonnet (Anthropic AI Assistant) Signed-off-by: Doug Hellmann <[email protected]>
|
Here are a few sample output files showing the difference between a full graph and a reduced graph: docling-graph.json.dot.pdf |
|
@dhellmann can you pass a graph file which has packages with customization, so that it would be easier for me to test the PR? |
6143e21 to
b9d5f47
Compare
Rename the --reduce command line option to --overrides-only to better reflect its purpose of filtering the dependency graph to show only nodes with fromager overrides. Update the help text to clarify that it includes nodes with settings, patches, or plugins, removing the redundant mention of "customizations" and "overrides" in the same description. Co-authored-by: Claude 3.5 Sonnet (Anthropic AI Assistant) Signed-off-by: Doug Hellmann <[email protected]>
b9d5f47 to
1fb5f94
Compare
The graph file itself doesn't record which packages had customizations, though we could add that. I'll upload the graph files I used to produce the PDFs but you'll have to run fromager with the context from our downstream configuration repo to get the same output. |
| # If the child doesn't have customizations, continue traversing | ||
| stack.append((child, new_path, current_req)) | ||
|
|
||
| return customized_nodes, reduced_dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will suggest that we break the reduce_graph() to smaller methods. Currently it is doing few things. Smaller functions will improve its readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would you break it up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dhellmann IMO we should merge the PR as it is and we can take up refactoring in a separate issue. I would be happy to work on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the idea I had (using Cursor), I have the code locally as well.
reduce_graph() function can be broken down into 4 focused helper functions:
-
_get_nodes_for_reduction() (9 lines)
Purpose: Determine starting node set based on install_only flag
Responsibility: Node selection logic -
_find_customized_nodes() (10 lines)
Purpose: Filter nodes to find only those with customizations
Responsibility: Node filtering logic -
_find_customized_dependencies_for_node() (33 lines)
Purpose: Complex graph traversal for a single node
Responsibility: Depth-first search algorithm
Most complex: Contains the core traversal logic -
_build_reduced_dependency_map() (8 lines)
Purpose: Orchestrate dependency mapping for all nodes
Responsibility: Coordinate traversal across all customized nodes -
reduce_graph() (Refactored, 13 lines)
Clear flow: Get nodes → Find customized → Build dependencies → Return
Simplify the logic for reducing a graph by moving the check into the PackageBuildInfo class.
9fc7a55 to
7eb87e9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good! I see there are few comments on MR so I will wait for them to be resolved before I hit approve
Add a new --reduce flag to the graph to-dot command that filters the
dependency graph to only include nodes with fromager customizations
(overrides, settings, patches, plugins, or pre-built wheels).
The reduced graph preserves dependency relationships by creating direct
edges between customized nodes, skipping intermediate nodes without
customizations. Original requirement specifications are tracked and
displayed in edge labels for better clarity.
Key changes:
This feature helps visualize which packages in a dependency tree actually
have fromager customizations while maintaining essential dependency
relationships.
Fixes #715
Cursor chat transcript: https://gist.github.com/dhellmann/e362954e00a7270dcc71037382f53853