search

BridgesUNCC / bridges-python

Python client library for Bridges
http://bridgesuncc.github.io
MIT License
2 stars 4 forks source link

Why not have add_edge and add_vertex return the new vertex/edge? #65

Open acbart opened 4 years ago

acbart commented 4 years ago

https://github.com/BridgesUNCC/bridges-python/blob/23938fa1085254cdd1c6958a2e0fb67b17ce9430/bridges/graph_adj_list.py#L78

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?

acbart commented 4 years ago

Just to comment further on the inconvenience: get_link_visualizer is quite painful to have to write out whenever I want to access the properties of an edge. I think the Graph API should be reconsidered to be easier to adjust settings. I was surprised I couldn't write things like:

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 add_* functions, you can update properties directly, and you can pass in more values to the functions.

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.

AlecGoncharow commented 4 years ago

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.

acbart commented 4 years ago

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++ :)

AlecGoncharow commented 4 years ago

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. https://pypi.org/project/bridges/3.1.0b1/

Your provided example code should just work with that new package.

acbart commented 4 years ago

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..!