-
-
Notifications
You must be signed in to change notification settings - Fork 307
Use dagree for interactive migration graphs #2670
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
✅ Deploy Preview for conda-forge-previews ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hum, it builds locally, investigating. |
|
The nodes colors now properly reflect the node status, the selected node provide a link to its PR (I don't do that for all nodes as it conflicts with the ability to select a sub-graph), it shows a warning if trying to render too many nodes (1000) even if 1500 works ok on my computer. Settings are now in a panel. |
|
@jaimergp you liked my other prs... this one is a bit bigger... (might need a rebase though, but i'm on my phone for now) |
|
Yea, this is a chunky one :D I did give it a try and works as (I guess) intended. I don't have the expertise in frontend to judge much, so my first reaction is to promote this PR in Zulip to see if maintainers find it useful for their work. I'll ping some folks so they help me evaluate the benefit:cost ratio and then decide if we consider it for review, ok? cc @h-vetinari, @danielnachun, @xhochy, @traversaro - wdyt? Preview link at https://deploy-preview-2670--conda-forge-previews.netlify.app/status/migration/?name=python314t (click on the Dependencies tab). |
|
I find it hard to figure out why X is not building yet/when it will build. With this I can view the family tree of X and see which package(s) i need to unblock. For me the scrolling and finding in the SVG is completely broken, and I have a hard time following links. This also helped me figure out that some of the packages on 314t were blocked on 314 as some parents were not "in-pr", which is super annoying to do in the table. of course all the option to add "done" and change the layout are likely overkill, i'm happy to remove them. I guess if this works, it could also help drop the dependency on graphviz in cf-script (maybe). |
Extracted from conda-forge#2670, should be less controversial, and easier to review.
|
#2679 extract just the part that make the SVG zoomable/panable without opening it in a new page. It should make review easier. I've push a used of the |
| {showSettings && ( | ||
| <div className={graphStyles.settingsPanel}> | ||
| <div className={graphStyles.toggleContainer}> | ||
| <label className={graphStyles.toggleLabel}> | ||
| <span>Include completed packages</span> | ||
| <input | ||
| type="checkbox" | ||
| className={graphStyles.toggleInput} | ||
| checked={showDoneNodes} | ||
| onChange={(e) => setShowDoneNodes(e.target.checked)} | ||
| /> | ||
| <span className={graphStyles.toggleSlider}></span> | ||
| </label> | ||
| </div> | ||
| <div className={graphStyles.settingsGrid}> | ||
| <div> | ||
| <label className={graphStyles.settingLabel}>Direction</label> | ||
| <select | ||
| id="graph-direction" | ||
| className={graphStyles.settingSelect} | ||
| value={graphDirection} | ||
| onChange={(e) => setGraphDirection(e.target.value)} | ||
| > | ||
| <option value="TB">Top to Bottom</option> | ||
| <option value="BT">Bottom to Top</option> | ||
| <option value="LR">Left to Right</option> | ||
| <option value="RL">Right to Left</option> | ||
| </select> | ||
| </div> | ||
| <div> | ||
| <label className={graphStyles.settingLabel}>Ranker</label> | ||
| <select | ||
| id="graph-ranker" | ||
| className={graphStyles.settingSelect} | ||
| value={graphRanker} | ||
| onChange={(e) => setGraphRanker(e.target.value)} | ||
| > | ||
| <option value="network-simplex">Network Simplex</option> | ||
| <option value="tight-tree">Tight Tree</option> | ||
| <option value="longest-path">Longest Path</option> | ||
| </select> | ||
| </div> | ||
| <div> | ||
| <label className={graphStyles.settingLabel}>Alignment</label> | ||
| <select | ||
| id="graph-align" | ||
| className={graphStyles.settingSelect} | ||
| value={graphAlign} | ||
| onChange={(e) => setGraphAlign(e.target.value)} | ||
| > | ||
| <option value="">Center (default)</option> | ||
| <option value="UL">Upper Left</option> | ||
| <option value="UR">Upper Right</option> | ||
| <option value="DL">Down Left</option> | ||
| <option value="DR">Down Right</option> | ||
| </select> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| )} |
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.
all of that is the setting panel, I'm happy to remove it if we agree on default.
|
Yeah, blue is unknown in the graph, if gray it would be confusing with other nodes. I can use --ifm-warning. I don't care about the |
|
Should I make |
|
There should now be a correct legend, and you can also point directly at a subgraph for a given package via the url: |
Really cool, that seems quite useful especially for large migrations that can't be rendered with the current graph. |
|
I like it. The graph in the example is quite complicated, but that's also the case where don't have any insight all now. For smaller migrations, I like to look at the graph and was sad in the past that it didn't work for the large ones. |
|
It starting to show its limit for ~1200 nodes for me, and I doubt the full graph is useful anyway. There are likely improvement we can do later; like maybe merging all nodes which have same status, parents and children. My guess is that there will be a ton of packages which are similar (e.g, many of the direct child node of numba on 314t). If it's fast/small enough we could maybe even who the (subgraph) on hover of a package name in the table, but I think this is for further discussions. Can someone kick the deploy ? Or do I push an empty commit ? |
The error is just the link checker, don't worry. Looks like folks like it, so I'll proceed with a more thorough review. |
* Make Migration SVG graph zoomable/panable. Extracted from #2670, should be less controversial, and easier to review. * Apply suggestions from code review Co-authored-by: jaimergp <jaimergp@users.noreply.github.com> * explicitely add d3 --------- Co-authored-by: jaimergp <jaimergp@users.noreply.github.com>
|
I merged 2679, but looks like we have conflicts now. |
In addition to the normal graphviz svg
dagree is already a dependency via mermaid/docusaurus
This filter and a remove packages that are "done", and/or have no
parrent children befor showing the graph, and split it into connected
components.
The interactive graph has the following features:
- span/zoom with mouse/trackpad
- hover a node to dimm all nodes except direct children/parents.
- click on a node to show only current graph and all parents (and
their parents...)/children (and their children...), but not
siblings.
A filter search bar to find a node easily and select it without having
to click on it.
A dropdown for the various dagree layout options I think are
un-necessary but let us explore the various dagree options.
The node color should reflect the ci status of the PR, but are not
properly clickable to open given PR, this is something I'm planning to
do if there is interest.
Note that there are some hacks as some packages are marked as "awaiting
parent", while they do not have any parent pending (like pyside2 for
314t, llvmlite for 314t also IIRC), maybe this should be fixed in the
table as well with a different label.
I'm also rendering the graph even if there is an extremly large number
of node, I think I there should be a confirmation if there is say > 1k
nodes and a confirmation, but one of my powerful machine handles things
really well so not sure.
This reverts commit 6e65d733e71fef7ce31a55273315b5d32e6bac94.
Add several improvements to the interactive dependency graph: - Display PR numbers as clickable links on selected nodes - Add collapsible settings panel with gear icon for graph controls - Implement CSS toggle switch for showing/hiding completed packages - Add large graph warning (threshold: 1000 nodes) to prevent browser slowdown - Improve node coloring based on migration category (bot-error, not-solvable, etc.) - Use border highlighting instead of fill color for selected nodes - Increase node padding for better readability The settings panel consolidates graph direction, ranker algorithm, alignment options, and the completed packages toggle into a single floating panel accessed via gear icon. Move inline styles to CSS module for better maintainability and remove unnecessary comments from the codebase.
This does miscellaneous fixes and improvements to the migration graph. - add legend, - fix double click to reset zoom - add url param for selected node for example `?name=python314t&dependency=trollimage``
Fixed, that was just your 2 line comments I accepted from the UI. Do you want me to squash ? |








PR Checklist:
docs/orcommunity/, you have added it to the sidebar in the corresponding_sidebar.jsonfileDisclaimer,
This makes heavy use of LLM, as I am not ususally touching javascript, and even less dagree. I did review and modify the code a lot so it is not just ask AI and see what sticks, even though there are still likely some cleaning needing in particular WRT to using CSS instead of inline style for d3 nodes.
If that's not ok, that's fine, I'm happy to close.
There are basically 2 things here in 2 commits:
I just want to know if I should push this further.
Suggestion to try this on the status/migration/?name=python314t, and search for example
pytorche-cpu, orlalappsit works on much bigger graph on my machine but can take a while to render/react.
Use dagree for interactive migration graph
In addition to the normal graphviz svg
dagree is already a dependency via mermaid/docusaurus
This filter and a remove packages that are "done", and/or have no
parrent children befor showing the graph, and split it into connected
components.
The interactive graph has the following features:
their parents...)/children (and their children...), but not
siblings.
A filter search bar to find a node easily and select it without having
to click on it.
A dropdown for the various dagree layout options I think are
un-necessary but let us explore the various dagree options.
The node color should reflect the ci status of the PR, but are not
properly clickable to open given PR, this is something I'm planning to
do if there is interest.
Note that there are some hacks as some packages are marked as "awaiting
parent", while they do not have any parent pending (like pyside2 for
314t, llvmlite for 314t also IIRC), maybe this should be fixed in the
table as well with a different label.
I'm also rendering the graph even if there is an extremely large number
of node, I think I there should be a confirmation if there is say > 1k
nodes and a confirmation, but one of my powerful machine handles things
really well so not sure.