Add Reducible for Eval and Id

[johnynek]

Using these on their own may not be that useful, but Reducible composes, so this allows us to use a reducible on F[Eval[T]], Eval[F[T]] or F[Id[T]], Id[F[T]].

One small nice win is that size on Eval can avoid evaluation entirely. Probably never a win in practice, but hey...

[johnynek]

should this be combined with the monad/comonad above?

I don't think we can ever implement Traverse[Eval] (I'd love to be wrong about that), so it didn't have the same ambiguous implicit concern that Id has (since Traverse[Id] also extends Foldable[Id], we don't want Reducible[Id] we have to either use priority, or unify them in the same instance).

[ceedubs]

Couldn't a fairly trivial Traverse[Eval] be implemented where traverse looks something like this?

def traverse[G[_]: Applicative, A, B](fa: Eval[A])(f: A => G[B]): G[Eval[B]] =
  f(fa.value).map(Eval.now)

It's a bit wonky, but the Comonad, Reducible, etc instances are already based on eager calls to .value, so I don't know if it's any different.

[ceedubs]

It occurs to me that such a Traverse instance can be defined for any comonad, so it it's probably a known thing with some known properties to people who know more than I do :)

[ceedubs]

It also occurs to me that a big difference between this Traverse instance and the instances that you've added is that yours shouldn't ever lead to stack overflows while this one is probably pretty likely to bite people with them.

[johnynek]

none of our instances call .value unless the return type requires it. It seems to me we should keep that since otherwise you lose the stack safety if Eval, so that is what I really meant. If you are willing to call .value, Eval is isomorphic to Id I think, but the best practice of Eval (though perhaps not required, but the real rule might somewhat complex and difficult to verify) is to only call .value once at the end.

[ceedubs]

I think that limiting ourselves to instances that (if they need to call .value at all) only call .value once at the end is probably a good general rule. 👍

[ceedubs]

Hmm sorry for all of the thinking out loud here, but I just realized that the traverse implementation above only calls .value once, just as the Reducible instances do. Is there a reason that we would consider this to be different? Maybe because the Eval in the return type suggests that we aren't calling .value?

[johnynek]

hit the flakey future test. Restarted.

[johnynek]

We could add Reducible for Function0 also.

[codecov-io]

Current coverage is 92.24% (diff: 96.87%)

Merging #1475 into master will increase coverage by <.01%

@@             master      #1475   diff @@
==========================================
  Files           242        246     +4   
  Lines          3619       3802   +183   
  Methods        3549       3683   +134   
  Messages          0          0          
  Branches         70        119    +49   
==========================================
+ Hits           3338       3507   +169   
- Misses          281        295    +14   
  Partials          0          0          

Powered by Codecov. Last update e20cb6b...544bfc1

[johnynek]

This many test misses would indicate Reducible and Foldable laws don't exercise all the functions.

[non]

@johnynek Yeah it seems like we need more of those. Do you mind adding some to the PR?

I'm 👍 on this addition, BTW.

[johnynek]

added some more laws to improve coverage. Should be good now.

I'm especially keen on the Reducible[Eval] since I think composing that will be somewhat useful.

[ceedubs]

Thanks @johnynek! I remember thinking about adding Reducible[Id] at some point and I can't remember why I didn't pursue that further.

I've left #1475 (comment) which we may want to make a conscious decision about before merging this.

[ceedubs]

👍

@non would you consider this good to merge?

I think that any instances that we've defined for Eval we should probably be able to to define for Function0 as well.

[johnynek]

Yes. You can't expect not to call Eval.value if the function returns the inner type. If we don't follow that rule, we need to remove Comonad also. So basically I would say if the function returns an Eval, no value should be called, otherwise it is okay.

…

On Thu, Dec 22, 2016 at 09:01 Cody Allen ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In core/src/main/scala/cats/Eval.scala <#1475>: > @@ -310,6 +310,30 @@ private[cats] trait EvalInstances extends EvalInstances0 { } } + implicit val catsReducibleForEval: Reducible[Eval] = Hmm sorry for all of the thinking out loud here, but I just realized that the traverse implementation above only calls .value once, just as the Reducible instances do. Is there a reason that we would consider this to be different? Maybe because the Eval in the return type suggests that we aren't calling .value? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1475>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEJdjdlP8-15Wdcos8JUoeA-u9Znw-rks5rKskNgaJpZM4K8sRX> .

[johnynek]

This seems to have 2 👍 unless I'm mistaken. @non gave a +1 early, would you (or someone else) like to look again before we merge for 0.9.0?

[non]

Yes! 👍