bitwalker / libgraph

A graph data structure library for Elixir projects
MIT License
524 stars 75 forks source link

Enables vertices to have more than one label #4

Closed MachinesAreUs closed 6 years ago

MachinesAreUs commented 7 years ago

I know it's possible to assign complex labels to vertices, like [:foo, :bar], but it would be nice if the handling of multiple labels could be internal to the library, since in several use cases you don't know beforehand all the labels for vertices.

This could work like this (modified and extended from doctests for Graph.label_vertex/3):

iex> g = Graph.new |> Graph.add_vertex(:a, :foo)
...> [:foo] = Graph.vertex_labels(g, :a)
...> g = Graph.label_vertex(g, :a, :bar)
...> Graph.vertex_labels(g, :a)
[:foo, :bar]

iex> g = Graph.new |> Graph.add_vertex(:a, [:foo, :bar])
...> Graph.vertex_labels(g, :a)
[:foo, :bar]

You could also receive both single terms and lists in Graph.label_vertex/3:

iex> g = Graph.new |> Graph.add_vertex(:a)
...> g = Graph.label_vertex(g, :a, [:foo, :bar])
...> Graph.vertex_labels(g, :a)
[:foo, :bar]

This implies 2 breaking changes:

I was careful not to break the .dot serialization (at least with the current tests), but maybe it is worth discussing including both the vertex name and labels in the generated .dot file.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.1%) to 94.775% when pulling 0d47b621392914b5d9be9d62df41d3f3a97867ab on MachinesAreUs:feature/multiple_labels_by_vertex into 391b24d4d77dece261d108bc862879b5e7fdfc57 on bitwalker:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.1%) to 94.775% when pulling 9b83eb74c661a739f6d6725a938ca007d02a209b on MachinesAreUs:feature/multiple_labels_by_vertex into 391b24d4d77dece261d108bc862879b5e7fdfc57 on bitwalker:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.06%) to 94.946% when pulling c79b5c00bee39b9a30b02aecd99ddc01f828b610 on MachinesAreUs:feature/multiple_labels_by_vertex into 391b24d4d77dece261d108bc862879b5e7fdfc57 on bitwalker:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.06%) to 94.946% when pulling 74733e25b8f3327fca5f48288bb5d56555c7c622 on MachinesAreUs:feature/multiple_labels_by_vertex into 391b24d4d77dece261d108bc862879b5e7fdfc57 on bitwalker:master.

MachinesAreUs commented 7 years ago

Hi @bitwalker. Any thoughts on this? :)

bitwalker commented 7 years ago

Sorry for the delay, I promise this is next on my list! Hopefully review this weekend and go from there :)

bitwalker commented 6 years ago

Can you rebase your changes on master? There are some changes in your PR that I've made on master, or that I would prefer not be in PRs (such as the version change), but other than that, it's pretty much ready to merge. Sorry for the long wait!

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.06%) to 94.955% when pulling 663b8538a8bd5241bfcd54babd22b9a22df89479 on MachinesAreUs:feature/multiple_labels_by_vertex into 3851394fb6bcdddfc094070ead03bc2970cd2c4e on bitwalker:master.

MachinesAreUs commented 6 years ago

Ready @bitwalker ! :)

bitwalker commented 6 years ago

Thanks! Since this is a breaking change, I'll release it as 1.0.0 sometime today.