update Extend and Comonad dependencies to match figure

[davidchambers]

Thank you for pointing out this inconsistency, @jlmorgan.

If Extend really does depend on Functor we should update the description of Extend rather than merge this pull request.

[SimonRichardson]

👍

…

On Fri, 16 Dec 2016, 23:15 David Chambers, ***@***.***> wrote: Thank you for pointing out this inconsistency, @jlmorgan <https://github.com/jlmorgan>. If Extend really *does* depend on Functor we should update the description of Extend rather than merge this pull request. ------------------------------ You can view, comment on, or merge this pull request online at: #213 Commit Summary - figures: remove Functor -> Extend from dependency graph File Changes - *M* README.md <https://github.com/fantasyland/fantasy-land/pull/213/files#diff-0> (2) - *M* figures/dependencies.dot <https://github.com/fantasyland/fantasy-land/pull/213/files#diff-1> (1) - *M* figures/dependencies.png <https://github.com/fantasyland/fantasy-land/pull/213/files#diff-2> (0) Patch Links: - https://github.com/fantasyland/fantasy-land/pull/213.patch - https://github.com/fantasyland/fantasy-land/pull/213.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#213>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACcaGDRfO34m_Y_5a5TtVAm3-9thMX4Kks5rIxufgaJpZM4LPrsp> .

[davidchambers]

Let's wait for confirmation from @joneshf before merging this pull request (he added the relationship in #192).

[rpominov]

But shouldn't Comonad depend on Functor? Spec says so.

[davidchambers]

Thanks, @rpominov! I've updated the pull request. :)

[safareli]

in PS class Functor w <= Extend w

[davidchambers]

That's instructive, @safareli. Shall we revert the changes to the diagram and instead move the reference to the Functor dependency in the spec from Comonad to Extend?

[safareli]

Shall we revert the changes to the diagram and instead move the reference to the Functor dependency in the spec from Comonad to Extend?

Extend is dual of Chain which is a Functor, also Extend holds values (it's kind is * -> *) so I think it should be a Functor, but lets see what @joneshf thinks.

[joneshf]

Yeah, we do want Functor -> Extend. We should make that explicit in the definition and move Comonads third law (w.extend(f) is equivalent to w.extend(x => x).map(f)) to Extend.

[davidchambers]

This is now an unrelated—but still intentional—change.

[davidchambers]

Please have another look, @joneshf.

[safareli]

btw the law with map is not present in PS Extend Comonad so do we have some special need for that?

[joneshf]

Hmm, good question. Maybe we don't need it. Though, then I wonder if we need Functor -> Extend if there's no laws between map and extend.

[jlmorgan]

Is there a way to implement Extend using map like how Chain can be implemented using:

chain(f) { return this.map(f).join(); }
join() { return this.value; }

Seems like you can't since map unpacks the value before supplying it to the f function.

[safareli]

@joneshf I don't think we should drop Functor ->, but we should take a look at variations of laws in haskell Comonad. looks like the map law we have is close to default definition of extend

extend f = fmap f . duplicate

if we substitute duplicate with extend id, we get:

extend f = fmap f . extend id

[joneshf]

@jlmorgan Sure.

extend(f) { return this.duplicate().map(f); }
duplicate() { return F(this); }

@safareli That law is only needed if you're defining both duplicate and extend. I don't know that we need it since our spec only mentions extend and not duplicate.

FWIW, I've found defining duplicate to be so much easier than defining extend for most data types. Maybe that's something to consider. Of course, we then lose the ability to derive map for a Comonad (similar to how it was when Traversable required sequence and not traverse). But, we're not even suggesting that right now. Seems like this PR is raising more issues than originally intended 😄.

[joneshf]

Maybe a worthwhile law is the second one here under duplicated: https://hackage.haskell.org/package/semigroupoids-5.1/docs/Data-Functor-Extend.html#v:duplicated

fmap (fmap f) . extend id = extend id . fmap f

Maybe that's just a free theorem?

[safareli]

Don't know but nor in HS.semigroupoids.Extend, nor in HS.comonad.Comonad and nor in PS.control.Extend we have the law. all of them have just extended f . extended g = extended (f . extended g) for case when extend is defined only. looks like it's not needed, so maybe we should remove it?

or we can just move the map law into Extend and create an issue to revisit the extend/comonad laws. @davidchambers in this case /laws should be updated too.

[davidchambers]

Could someone propose appropriate names for the Extend and Comonad laws? When we moved a law from Comonad to Extend we ended up with two Extend laws both described as "associativity".

[safareli]

That law is not even in cats CoflatMap.

I think we should just remove it, and if we in future have some way in the spec to say that for example Extend should define extend or duplicate then we should revisit laws for extend and comonad

[jlmorgan]

Shouldn't everything that derives from Functor observe the same laws? Should you not be able to do:

e.extend(a => a) is equivalent to e (identity)
e.extend(x => f(g(x))) is equivalent to e.extend(g).extend(f) (composition)

You could add in Chain's associativity:

e.extend(f).extend(g) is equivalent to e.extend(x => f(x).extend(g)) (associativity)

Personally, I think having the companion functions like duplicate and join provide better clarity as to how both Chain and Extend are derived from Functor's map. Providing how each of Functor's descendants are implemented using map provides not only clarity, but a glimpse into intended uses of the morphisms.

Edit: After some testing with the following method setup:

class Thing {
  constructor(v) { this.value = v; }
  map(f) { return new Thing(f(this.value)); }
  join() { return this.value; }
  chain(f) { return this.map(f).join(); }
  duplicate() { return new Thing(this); }
  extend(f) { return this.duplicate().map(f); }
  toString() { return `[${this.constructor.name} ${this.value}]`; }
}

const compose = f => g => x => f(g(x));
const id = x => x;
const e = new Thing(1);
const f = id;
const g = id;

console.log(
  "identity",
  e.extend(id).toString(),
  "should equal",
  e.toString()
);
console.log(
  "composition",
  e.extend(compose(f)(g)).toString(),
  "should equal",
  e.extend(f).extend(g).toString()
);
console.log(
  "associativity",
  e.extend(f).extend(g).toString(),
  "should equal",
  e.extend(x => f(x).extend(g)).toString()
);

I get the following:

identity: [Thing [Thing 1]] should equal [Thing 1]
composition: [Thing [Thing 1]] should equal [Thing [Thing [Thing 1]]]
associativity: [Thing [Thing [Thing 1]]] should equal [Thing [Thing [Thing 1]]]

Associativity definitely worked. = )

This is likely due to my lack of understanding. 😆

Edit 2:
Before having the companion methods, I would expected the following method signatures to be passed as an argument to the respective methods:

map's (f) :: (a -> a)
chain's (f) :: (a -> Chain a)
extend's (f) :: (Extend a -> a)

With the companion methods, it seems more like I just need a -> a for all of them since join removes a box added by map, but it seems like duplicate is just adding a box and then map is re-boxing the outer box…

[joneshf]

That law is not even in cats CoflatMap.

It's coherence: https://github.com/typelevel/cats/blob/155f7f534993c30d6e757de990330ac796dad5da/laws/src/main/scala/cats/laws/CoflatMapLaws.scala#L19-L20

I think we should just remove it, and if we in future have some way in the spec to say that for example Extend should define extend or duplicate then we should revisit laws for extend and comonad

Sounds good to me.

[joneshf]

@jlmorgan "identity" and "composition" as you've defined them there should not work, as you've shown. If they did work, then extend would just be map, and "associativity" would not work as you've defined it. In other words, there'd be no purpose for defining the Extend algebra; it would just be Functor again with a different name.

[davidchambers]

I've removed the law I believe @safareli and @joneshf suggested should be removed. Let me know if everything is now in order. :)

[safareli]

@joneshf the "coherence" uses coflatten.
@davidchambers 👍

[joneshf]

@safareli I'm confused. What are we talking about?

[safareli]

That law is not even in cats CoflatMap.

It's coherence: https://github.com/typelevel/cats/blob/155f7f534993c30d6e757de990330ac796dad5da/laws/src/main/scala/cats/laws/CoflatMapLaws.scala#L19-L20

What i'm saying is that the coherence law also depends on coflatten (duplicate) which we don't have here.

[davidchambers]

Shall we merge this pull request, @joneshf?

[SimonRichardson]

😂

…

On Tue, 20 Dec 2016, 20:01 David Chambers, ***@***.***> wrote: Shall we merge this pull request, @joneshf <https://github.com/joneshf>? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#213 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACcaGDPH6hho5lcOSqBM4KAvYuwaYf6vks5rKDQqgaJpZM4LPrsp> .