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

merge semantics is not clear #57

Closed
osa1 opened this issue Oct 24, 2016 · 1 comment
Closed

merge semantics is not clear #57

osa1 opened this issue Oct 24, 2016 · 1 comment
Assignees

Comments

@osa1
Copy link

osa1 commented Oct 24, 2016

Documentation says this:

  -- | Merge the 'Context' into the 'DynGraph'.
  --
  --   Contexts should only refer to either a Node already in a graph
  --   or the node in the Context itself (for loops).

Without looking at an implementation it's not clear to me what what does "merge" mean in this context. I think this could use more sentences. More specifically, I think it's worth adding that merge is for adding new stuff and updating labels, and it's not possible to remove an edge using it. Also, when a new edge A -> B added using this, predecessors of B is automatically updated.

Since DynGraph is a typeclass I think these should be specified so that using a different implementation would not break the program

@ivan-m ivan-m self-assigned this Oct 26, 2016
@ivan-m
Copy link
Contributor

ivan-m commented Oct 26, 2016

Well, it's for adding a node and edges associated with it, not for updating labels.

I'll have a look, but I'm not sure how much clearer this can be; some fundamental knowledge as to how inductive graphs work is required, and from the definition of a Context it should be clear that it can't remove any existing edges.

@ivan-m ivan-m closed this as completed in 1b08580 Feb 15, 2017
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