Add function to animate graph routes #1285
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1285 +/- ##
==========================================
+ Coverage 98.44% 98.46% +0.02%
==========================================
Files 24 24
Lines 2383 2417 +34
==========================================
+ Hits 2346 2380 +34
Misses 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| G: nx.MultiDiGraph, | ||
| route: list[int], | ||
| *, | ||
| route_color: str = "r", | ||
| route_linewidth: float = 4, | ||
| route_alpha: float = 0.5, | ||
| orig_dest_size: float = 100, | ||
| ax: Axes | None = None, | ||
| **pg_kwargs: Any, # noqa: ANN401 |
There was a problem hiding this comment.
Does the user need more control than this? Animation speed? Repetition? Would fine-grain control be more easily exposed via keyword arguments to pass to matplotlib? Or?
There was a problem hiding this comment.
Yeah that makes sense, IMO it's worth adding the two you mentioned (speed and repetition) as explicitly defined/documented arguments here, then additionally allow passing arbitrary kwargs to matplotlib for more advanced/obscure things.
There was a problem hiding this comment.
We should keep the function signature/arguments as parsimonious and streamlined as possible. If these are straightforward kwargs to pass directly to matplotlib, that's probably the best solution.
There was a problem hiding this comment.
The issue I'm running into with this is that we can only gobble up one dict of arbitrary kwargs, but then there are several places in this function where they may need to be passed: some may need to go to plot_graph (by way of _prepare_graph_route_plot), others may need to go to the constructor of FuncAnimation, others may need to go to Animation.save's savefig_kwargs argument, and finally some may need to be passed directly to Animation.save. So I feel like I need to split them all out to avoid passing unexpected kwargs in the wrong spot ... something like this nkleinbaer@904ecda ... but that feels kinda hacky and difficult to maintain (not to mention I haven't tested out that passing any of these kwargs actually works as expected, that's just a draft). What are your thoughts?
There was a problem hiding this comment.
@nkleinbaer perhaps each of the constant kwarg dicts you define in that commit at lines 404-460 could instead be individual function arguments of type dict. Then the user could pass individual kwarg dicts for each destination function.
|
Hello, I think this feature should be implemented, it works well and looks good. I have just found one thing that could be improved by adding support for Jupyter Lab, with in the animate_graph_route function: if pg_kwargs.get("html_animation", True):
From IPython.display import HTML
HTML(anim.to_html5_video())Or return the anim variable so we can do it outside. Another possible idea is to be able to animate multiple routes at the same time. Sorry if I'm not using GitHub correctly. |
|
@olivier-be thanks for your feedback. I agree we should make this useful for Jupyter users. We won't want to add an |
Opening a PR to implement feature proposed in #1187. Adds a new function
animate_graph_routethat is analogous toplot_graph_route, but shows a edge-by-edge animation instead of a static image.In 59fd139 I simply copied the existing
plot_graph_routefunction and modified it to create an animation. Since this created duplication of code shared by the two funcs (i.e. plotting the graph itself and origin/destination nodes) I then added a 52ffb46 to refactor this out. Wanted to keep that as a separate commit commit so it's easy to drop per your advice (thinking perhaps you may want to avoid touching existing functionality?).For testing just added a single call to the new function in
test_routing, following from what I saw done forplot_graph_route. Can add more tests if desired.Haven't updated the changelog yet, not sure if I should put this under the
2.0.2section or a new section.Example usage (adapted from this example):