OmenApps / django-postgresql-dag

Directed Acyclic Graphs with a variety of methods for both Nodes and Edges, and multiple exports (NetworkX, Pandas, etc). This project is the foundation for a commercial product, so expect regular improvements. PR's and other contributions are welcomed.
Apache License 2.0
41 stars 6 forks source link

Unable to delete children/parents when they share more than one edge #3

Closed mheppner closed 3 years ago

mheppner commented 3 years ago

Great library, this is working pretty well for us!

One issue I'm facing is that you can add multiple duplicate edges, resulting in some instability in the API.

node = GraphNode.objects.get(id=1)
new_parent = GraphNode.objects.get(id=2)

node.add_parent(new_parent)

# can call this multiple times
node.add_parent(new_parent)
node.add_parent(new_parent)

# need to refresh the related manager
node.refresh_from_db(fields=['parents'])
print(node.parents)

This results in 3 edges being created, all with the same parent and child pointers. This may be desired - it's still a valid graph. The issue comes when you try to delete the parent:

node = GraphNode.objects.get(id=1)
parent_to_remove = GraphNode.objects.get(id=2)

node.remove_parent(parent_to_remove)

An exception is raised:

Exception Type: MultipleObjectsReturned
Exception Value: get() returned more than one GraphEdge -- it returned 3!

Should remove_parent() be changed from parent.children.through.objects.get(parent=parent, child=self).delete() to a .filter()?

JackAtOmenApps commented 3 years ago

@mheppner - thanks, great catch. Will update this shortly and add testing for that case as well.

I have a bit of spare time, so expect to see several additional improvements over the next couple weeks (github actions, more thorough testing, updated docs). If you spot any other issues or have feature requests that might make this package better for everyone, please let me know.

mheppner commented 3 years ago

much thanks! 💯

JackAtOmenApps commented 3 years ago

Resolved in 0.1.9 with additional tests to check for deletion of parent of child when they share multiple edges.

I renamed this issue, and added #4 which I'll work on this week to add a non-breaking option to prevent any node from having multiple shared edges with a parent/child node.