-
Notifications
You must be signed in to change notification settings - Fork 4
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
Why not have add_edge and add_vertex return the new vertex/edge? #65
Comments
Just to comment further on the inconvenience: graph = GraphAdjList()
# def add_vertex(id: str, label: str=None, color: str= "blue", data: Any) -> Element:
home = graph.add_vertex("Home", color="red")
neighbor = graph.add_vertex("Neighbor 1", "First")
# def add_edge(src: Union[str, Element], dest: Union[str, Element], label: str=None, color: str="blue", thickness: float=1.0, data: Any) -> Edge:
new_edge = graph.add_edge(home, neighbor)
back_edge = graph.add_edge(neighbor.id, home.id, "27", "green")
back_edge.thickness = 2.0
my_edge = graph.get_edge(home, neighbor)
my_edge.label = "22" So you could create edges using either vertices or IDs, you can get them from the I'm also not clear what the advantage is from arbitrarily separating certain visual properties of edges from other properties (e.g., what's part of the Edge and what's part of the Link). Perhaps it makes sense internally, but the external interface now has two distinct concepts to introduce to students. |
This has been discussed internally years ago, the main reasoning was we wanted to keep a consistent API and C++ does not like returning values by default if you aren't planning to store the value in a variable. I agree the API is much nicer in this form, I will bring it up with the team. |
I appreciate the simplicity of having a unified codebase across languages, but if I didn't mind teaching with an awful API than I would just teach C++ :) |
A new beta package has been pushed with some of these API changes and the fix with the wrong link visualizer being sent to the server. Your provided example code should just work with that new package. |
Excellent timing! My students will be starting with my project tomorrow, so there's just enough time for me to update instructions. I'll let you know how it goes..! |
bridges-python/bridges/graph_adj_list.py
Line 78 in 23938fa
It's quite inconvenient to have to use other methods to access newly created vertices and edges, especially since I can't pass in label/color/thickness settings via these functions. Why not have these functions at least return something?
The text was updated successfully, but these errors were encountered: