Add docstring to GenericGraph.__getitem__() - #4761
Conversation
Allows accessing edges via tuple keys like g[(1, 2)] in addition to vertex lookups like g[1]. Previously only vertex lookups were supported. Fixes ManimCommunity#3798
chopan050
left a comment
chopan050
left a comment
There was a problem hiding this comment.
Thanks for implementing this! There's one thing that we need to address:
| def __getitem__(self: Graph, v: Hashable | tuple[Hashable, ...]) -> Mobject: | ||
| """Get a vertex or edge by its name/identifier. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| v | ||
| A vertex name (hashable) or an edge tuple ``(u, v)``. | ||
|
|
||
| Returns | ||
| ------- | ||
| Mobject | ||
| The :class:`~.Mobject` corresponding to the given vertex or edge. | ||
|
|
||
| Raises | ||
| ------ | ||
| KeyError | ||
| If ``v`` is not a valid vertex or edge. | ||
| """ | ||
| if isinstance(v, tuple): | ||
| return self.edges[v] | ||
| return self.vertices[v] |
There was a problem hiding this comment.
One thing I mentioned in issue #3798 is that vertices can be any hashable object. This includes tuples: vertices can be tuples themselves. This is not too strange, actually. For example, vertices could represent 2D coordinates. There's also this concept of a "line graph L of a graph G" where each vertex of L is an edge of G: if there is an edge connecting vertices 1 and 3 of G, then L has a vertex (1, 3), and so on. networkx actually implements this.
It is possible that the graph contains two vertices, say, (1, 2) and (1, 3), and an edge connecting both: ((1, 2), (1, 3)). Thus, verifying that v is a tuple is not enough to determine whether it represents an edge or a vertex.
With the current code, if graph is the example graph from above and you try to index graph[(1, 2)], it will attempt to find an edge between vertices 1 and 2. Since that edge and those vertices do not exist, this will raise an error, even though there exists a vertex (1, 2).
One way I would solve this is by instead asking whether v is contained in the vertices or the edges before attempting to retrieve the Mobject:
if v in self.vertices:
return self.vertices[v]
if v in self.edges:
return self.edges[v]
raise IndexError(f"Vertex or edge {v} not found")
# or some more elaborated message, like
# "Couldn't find vertex {v} or edge connecting vertices {v[0]} and {v[1]}"
# when v is a tuple of 2 elements
chopan050
changed the title
GenericGraph.__getitem__()
3 participants
EDIT by chopan050: an older PR #3799 was merged which allowed indexing edges of a graph, but the docstring in this PR is useful.
Description
Allows accessing edges via tuple keys like g[(1, 2)] in addition to vertex lookups like g[1]. Previously only vertex lookups were supported.
Fixes #3798
Changes