Skip to content
This repository has been archived by the owner on Mar 11, 2020. It is now read-only.

Fix GDecomp#redecorate #26

Closed
wants to merge 1 commit into from
Closed

Conversation

adelbertc
Copy link
Contributor

@adelbertc adelbertc commented Sep 17, 2017

  • Redecoration includes the focus of the decomposition when acting on
    the rest of the graph
  • Fix GDecomp generator so incorrect instances cannot be built (this
    should probably be a smart constructor)

/cc @runarorama

- Redecoration includes the focus of the decomposition when acting on
the rest of the graph
- Fix GDecomp generator so incorrect instances cannot be built (this
should probably be a smart constructor)
@adelbertc adelbertc mentioned this pull request Sep 17, 2017
@adelbertc
Copy link
Contributor Author

Some notes:

  • The new redecorate fails comonad laws without my fix to the generator
  • Both the old and new redecorate pass the comonad laws fine (at least it hasn't failed on a couple of runs on my machine)
  • The old one kinda makes more sense since you're already focused on a particular node, it's a bit weird to re-attach the focused node before running the action on the rest of the graph
  • At the same time the new one is useful and "consistent", running an action on each node given the decomposition of the graph with respect to it (see: http://blog.higher-order.com/blog/2016/04/02/a-comonad-of-graph-decompositions/)

@joroKr21
Copy link
Contributor

GraphGen.genGDecomp is not fixed properly. It can still generate a graph that doesn't contain the vertex (or the adjacent nodes) in the context. Here is a proper solution:

  def genGDecomp[N: Arbitrary, A: Arbitrary, B: Arbitrary]: Gen[GDecomp[N, A, B]] = for {
    g <- graphGen[N, A, B] if !g.isEmpty
    n <- Gen.oneOf(g.nodes)
  } yield g.decomp(n).toGDecomp.get

This was referenced Sep 25, 2017
@rossabaker
Copy link
Contributor

Thanks! Folded into #27 so we can minimize release churn.

@rossabaker rossabaker closed this Sep 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants