Add edge-key support in `GenericGraph.__getitem__()`
Motivation and Explanation: Why and how do your changes improve the library?
In the old GenericGraph, you can get the vertex 1 via g[1], but you cannot get the edge 1->2 via g[(1, 2)]. I think this is a little
confusing, so I add this support. See #3798.
Links to added or changed documentation pages
https://docs.manim.community/en/stable/reference/manim.mobject.graph.Graph.html#movingvertices: I saw the g[1] in this documentation. It's worth to consider add some examples (e.g. g[(1, 2)].animate.set_color(RED)).
Reviewer Checklist
- [ ] The PR title is descriptive enough for the changelog, and the PR is labeled correctly
- [ ] If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
- [ ] If applicable: newly added functions and classes are tested
I would appreciate it if you could add some examples to the documentation, so that people know how this works.
Great idea! I'll update the documentation later.
At some point when I wrote the __getitem__-interface for graphs, I wanted to lean on the mathematical notation for induced subgraphs. In other words, I'd have liked to implement that for a graph G, the expression G[v1, v2, v3] should return the subgraph induced by the vertices v1, v2, v3.
However, given that G[v1] currently also returns a vertex, and not a Graph with a single vertex, I think I should abandon my original idea, the proposed access to edges is a good idea.
The current implementation needs to be improved: Vertices can also be tuples, it is not safe to assume that when a tuple is fed to the graph that it will correspond to an edge:
verts = [(i, j) for i in range(4) for j in range(4)]
edges = [((0, 0), (0, 1)), ((0, 0), (1, 0)), ((1, 1), (1, 0)), ((1, 1), (0, 1)), ((1, 1), (1, 2)), ((1, 1), (2, 1)), ...]
G = Graph(verts, edges)
At some point when I wrote the
__getitem__-interface for graphs, I wanted to lean on the mathematical notation for induced subgraphs. In other words, I'd have liked to implement that for a graphG, the expressionG[v1, v2, v3]should return the subgraph induced by the verticesv1, v2, v3.However, given that
G[v1]currently also returns a vertex, and not a Graph with a single vertex, I think I should abandon my original idea, the proposed access to edges is a good idea.
As far as I know, the GenericGraph use networkx to logically represent a graph. Maybe you can use this for some operation of graphs like getting induced subgraphs.
The current implementation needs to be improved: Vertices can also be tuples, it is not safe to assume that when a tuple is fed to the graph that it will correspond to an edge...
Good question! And it's easy to think of plan A that using isinstance() to check it's a edge-key or vertex-key. But when unfortunately vertice is ugly like vertice = [1, 2, (1, 2)], it requires a also ugly code for check.
Plan B is to use in keyword. I mean:
- If
k in self.vertice, thenkis a vertex-key. - Else if
k in self.edges, thenkis a edge-key. - Else raise a
KeyError
Step 1 and 2 can be swapped for real implementation. It's clean and transparent and I thought it's behavior can be expected.
I prefer plan B and I look forward to your suggestions.🤗
I would prefer to let @behackl review this, as I haven't used graphs enough to look at the use cases :)
Hi @behackl, I'd like to invite you to review my fix. Feel free to comment🥰