aysylu / loom

Graph library for Clojure. Mailing list https://groups.google.com/forum/#!forum/loom-clj
http://aysy.lu/loom/
887 stars 108 forks source link

when nodes are records, loom.io/view renders incorrectly #75

Closed naipmoro closed 8 years ago

naipmoro commented 9 years ago
; this renders correctly
(def g (graph/digraph [-1 0] [2 0] [0 1] [4 1]))

; but
(defrecord rec [n])
(def g2 (graph/digraph [(rec. -1) (rec. 0)] [(rec. 2) (rec. 0)]
                       [(rec. 0) (rec. 1)] [(rec. 4) (rec. 1)]))

; the rendered graph below combines 2 nodes into 1
(io/view g2)
tessellator commented 8 years ago

I would argue that this isn't a bug in loom. Loom assumes (by default) that the str function returns a unique string for a unique input value when building a dot string to render, and I think that's pretty reasonable. Your example demonstrates a case in which that assumption breaks down.

For the case that you provide, the implementation of loom.io/dot-str eventually calls str on each node (see here). It just so happens that (= (str (rec. 0)) (str (rec. -1))) => true, so you end up specifying a relation from a node to itself in the dot string output. (g2 is still defined correctly.)

As an aside, it's also true that each pair for an additional step away from zero holds the relationship, so (= (str (rec. 1)) (str (rec. -2))) => true, &c. I have tested this with Clojure 1.7 and 1.8. ClojureScript outputs the string [object Object] for every case, so the strings for all cases are equal.

Personally, I consider this an interesting, but minor, edge case that falls outside the purview of loom. (Note that if you use strings instead of numbers to build up your records, the output appears as expected.)

The way to deal with this is to either provide your own implementation of toString that outputs a unique string for each record or provide a label function that serves the same purpose when you call view - something like (io/view g2 :node-label :n).

pataprogramming commented 8 years ago

I agree with @tessellator's assessment (and, thanks, @tessellator!). GraphViz is, for better or worse, string-driven. This makes it easy to write interfaces for from other languages, at the loss of some insight into the source language's specifics. Either of the suggested approaches are reasonable workarounds for this behavior.

The documentation is a bit weak in this area, and does require some knowledge of the way GraphViz works. I've made a note to expand on this and add a caveat in the docs.