Skip to content
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

exportviaShow might collapse vertices where (show v1) == (show v2) but v1 ≠ v2 #53

Closed
jmtd opened this issue Mar 12, 2018 · 2 comments

Comments

@jmtd
Copy link

jmtd commented Mar 12, 2018

Consider

import Algebra.Graph
import Algebra.Graph.Export.Dot
data SillyList = Slist Int Char deriving (Ord,Eq)
instance Show SillyList where
    show (Slist _ c) = show c
v0 = (Slist 0 'a')
v1 = (Slist 1 'a')
v2 = (Slist 2 'b') 
v3 = (Slist 3 'c')
g = path [v0,v1,v2,v3]

exportViaShow g results in:

err

But we want

yes

In graphviz language this is achieved by unique node IDs but labelling them with the show value e.g. 0 [label="a"] (although the hard part here is choosing unique node ID values per vertex for arbitrary types)

@snowleopard
Copy link
Owner

I'd say this is the right behaviour. Your instance Show breaks the unstated (but generally agreed upon) requirement for Show instances: if show x == s then it must be possible to feed s to a Haskell compiler and obtain x back. This might seem like a strange requirement, but it's very useful when you want to show a Haskell expression and use the result as a real Haskell code.

This requirement is often violated, but I'd advise against this, because some useful libraries (for example, Facebook's Haxl) rely on it to generate Haskell code.

Note that a consequence of this requirement is that show should be an injective (or one-to-one) function, which guarantees that this issue will not occur.

If you want to obtain the second graph, you need to use the export function instead, where you are in full control of how labels are assigned.

@snowleopard
Copy link
Owner

@jmtd I'm convinced the current behaviour is correct, so I'm closing this issue. Feel free to reopen if you'd like to argue that something should be changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants