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.

Sign up for GitHub

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

Jump to bottom

Add Reducible for Eval and Id #1475

Merged
merged 2 commits into from
Jan 3, 2017
Merged

Add Reducible for Eval and Id #1475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
97c4c87
Add Reducible for Eval and Id
johnynek Nov 25, 2016
544bfc1
Add more ReducibleLaws to improve coverage
johnynek Dec 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions core/src/main/scala/cats/Eval.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,30 @@ private[cats] trait EvalInstances extends EvalInstances0 {
}
}

implicit val catsReducibleForEval: Reducible[Eval] =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

new Reducible[Eval] {
def foldLeft[A, B](fa: Eval[A], b: B)(f: (B, A) => B): B =
f(b, fa.value)
def foldRight[A, B](fa: Eval[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] =
fa.flatMap(f(_, lb))

override def reduce[A](fa: Eval[A])(implicit A: Semigroup[A]): A =
fa.value
override def reduceLeft[A](fa: Eval[A])(f: (A, A) => A): A =
fa.value
def reduceLeftTo[A, B](fa: Eval[A])(f: A => B)(g: (B, A) => B): B =
f(fa.value)
override def reduceRight[A](fa: Eval[A])(f: (A, Eval[A]) => Eval[A]): Eval[A] =
fa
def reduceRightTo[A, B](fa: Eval[A])(f: A => B)(g: (A, Eval[B]) => Eval[B]): Eval[B] =
fa.map(f)
override def reduceRightOption[A](fa: Eval[A])(f: (A, Eval[A]) => Eval[A]): Eval[Option[A]] =
fa.map(Some(_))
override def reduceRightToOption[A, B](fa: Eval[A])(f: A => B)(g: (A, Eval[B]) => Eval[B]): Eval[Option[B]] =
fa.map { a => Some(f(a)) }
override def size[A](f: Eval[A]): Long = 1L
}

implicit def catsOrderForEval[A: Order]: Order[Eval[A]] =
new Order[Eval[A]] {
def compare(lx: Eval[A], ly: Eval[A]): Int =
Expand Down
20 changes: 18 additions & 2 deletions core/src/main/scala/cats/package.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ package object cats {
* encodes pure unary function application.
*/
type Id[A] = A
implicit val catsInstancesForId: Bimonad[Id] with Monad[Id] with Traverse[Id] =
new Bimonad[Id] with Monad[Id] with Traverse[Id] {
implicit val catsInstancesForId: Bimonad[Id] with Monad[Id] with Traverse[Id] with Reducible[Id] =
new Bimonad[Id] with Monad[Id] with Traverse[Id] with Reducible[Id] {
def pure[A](a: A): A = a
def extract[A](a: A): A = a
def flatMap[A, B](a: A)(f: A => B): B = f(a)
Expand All @@ -48,6 +48,22 @@ package object cats {
f(a, lb)
def traverse[G[_], A, B](a: A)(f: A => G[B])(implicit G: Applicative[G]): G[B] =
f(a)
override def reduce[A](fa: Id[A])(implicit A: Semigroup[A]): A =
fa
def reduceLeftTo[A, B](fa: Id[A])(f: A => B)(g: (B, A) => B): B =
f(fa)
override def reduceLeft[A](fa: Id[A])(f: (A, A) => A): A =
fa
override def reduceLeftToOption[A, B](fa: Id[A])(f: A => B)(g: (B, A) => B): Option[B] =
Some(f(fa))
override def reduceRight[A](fa: Id[A])(f: (A, Eval[A]) => Eval[A]): Eval[A] =
Now(fa)
def reduceRightTo[A, B](fa: Id[A])(f: A => B)(g: (A, Eval[B]) => Eval[B]): Eval[B] =
Now(f(fa))
override def reduceRightToOption[A, B](fa: Id[A])(f: A => B)(g: (A, Eval[B]) => Eval[B]): Eval[Option[B]] =
Now(Some(f(fa)))
override def reduceMap[A, B](fa: Id[A])(f: A => B)(implicit B: Semigroup[B]): B = f(fa)
override def size[A](fa: Id[A]): Long = 1L
}

type Eq[A] = cats.kernel.Eq[A]
Expand Down
19 changes: 19 additions & 0 deletions laws/src/main/scala/cats/laws/ReducibleLaws.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,30 @@ trait ReducibleLaws[F[_]] extends FoldableLaws[F] {
): IsEq[B] =
fa.reduceMap(f) <-> fa.reduceRightTo(f)((a, eb) => eb.map(f(a) |+| _)).value

def reduceRightToConsistentWithReduceRightToOption[A, B](
fa: F[A],
f: A => B
)(implicit
B: Semigroup[B]
): IsEq[Option[B]] =
fa.reduceRightToOption(f)((a, eb) => eb.map(f(a) |+| _)).value <->
fa.reduceRightTo(f)((a, eb) => eb.map(f(a) |+| _)).map(Option(_)).value

def reduceRightConsistentWithReduceRightOption[A](fa: F[A], f: (A, A) => A): IsEq[Option[A]] =
fa.reduceRight((a1, e2) => Now(f(a1, e2.value))).map(Option(_)).value <->
fa.reduceRightOption((a1, e2) => Now(f(a1, e2.value))).value

def reduceReduceLeftConsistent[B](fa: F[B])(implicit B: Semigroup[B]): IsEq[B] =
fa.reduce <-> fa.reduceLeft(B.combine)

def traverseConsistent[G[_]: Applicative, A, B](fa: F[A], f: A => G[B]): IsEq[G[Unit]] =
fa.traverse1_(f) <-> fa.traverse_(f)

def sequenceConsistent[G[_]: Applicative, A](fa: F[G[A]]): IsEq[G[Unit]] =
fa.sequence1_ <-> fa.sequence_

def sizeConsistent[A](fa: F[A]): IsEq[Long] =
fa.size <-> fa.reduceMap(_ => 1L)
}

object ReducibleLaws {
Expand Down
14 changes: 13 additions & 1 deletion laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package cats
package laws
package discipline

import cats.instances.option._
import cats.instances.long._

import org.scalacheck.{Arbitrary, Cogen}
import org.scalacheck.Prop.forAll

Expand All @@ -10,10 +13,12 @@ trait ReducibleTests[F[_]] extends FoldableTests[F] {

def reducible[G[_]: Applicative, A: Arbitrary, B: Arbitrary](implicit
ArbFA: Arbitrary[F[A]],
ArbFB: Arbitrary[F[B]],
ArbFGA: Arbitrary[F[G[A]]],
ArbGB: Arbitrary[G[B]],
CogenA: Cogen[A],
EqG: Eq[G[Unit]],
EqA: Eq[A],
EqB: Eq[B],
MonoidB: Monoid[B]
): RuleSet =
Expand All @@ -22,8 +27,15 @@ trait ReducibleTests[F[_]] extends FoldableTests[F] {
parent = Some(foldable[A, B]),
"reduceLeftTo consistent with reduceMap" -> forAll(laws.reduceLeftToConsistentWithReduceMap[A, B] _),
"reduceRightTo consistent with reduceMap" -> forAll(laws.reduceRightToConsistentWithReduceMap[A, B] _),
"reduceRightTo consistent with reduceRightToOption" ->
forAll(laws.reduceRightToConsistentWithReduceRightToOption[A, B] _),
"reduceRight consistent with reduceRightOption" ->
forAll(laws.reduceRightConsistentWithReduceRightOption[A] _),
"reduce consistent with reduceLeft" ->
forAll(laws.reduceReduceLeftConsistent[B] _),
"traverse1_ consistent with traverse_" -> forAll(laws.traverseConsistent[G, A, B] _),
"sequence1_ consistent with sequence_" -> forAll(laws.sequenceConsistent[G, A] _)
"sequence1_ consistent with sequence_" -> forAll(laws.sequenceConsistent[G, A] _),
"size consistent with reduceMap" -> forAll(laws.sizeConsistent[A] _)
)
}

Expand Down
5 changes: 4 additions & 1 deletion tests/src/test/scala/cats/tests/EvalTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package tests

import scala.math.min
import cats.laws.ComonadLaws
import cats.laws.discipline.{BimonadTests, CartesianTests, MonadTests, SerializableTests}
import cats.laws.discipline.{BimonadTests, CartesianTests, MonadTests, ReducibleTests, SerializableTests}
import cats.laws.discipline.arbitrary._
import cats.kernel.laws.{GroupLaws, OrderLaws}

Expand Down Expand Up @@ -99,6 +99,9 @@ class EvalTests extends CatsSuite {
checkAll("Bimonad[Eval]", SerializableTests.serializable(Bimonad[Eval]))
checkAll("Monad[Eval]", SerializableTests.serializable(Monad[Eval]))

checkAll("Eval[Int]", ReducibleTests[Eval].reducible[Option, Int, Int])
checkAll("Reducible[Eval]", SerializableTests.serializable(Reducible[Eval]))

checkAll("Eval[Int]", GroupLaws[Eval[Int]].group)

{
Expand Down
3 changes: 3 additions & 0 deletions tests/src/test/scala/cats/tests/IdTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,7 @@ class IdTests extends CatsSuite {

checkAll("Id[Int]", TraverseTests[Id].traverse[Int, Int, Int, Int, Option, Option])
checkAll("Traverse[Id]", SerializableTests.serializable(Traverse[Id]))

checkAll("Id[Int]", ReducibleTests[Id].reducible[Option, Int, Int])
checkAll("Reducible[Id]", SerializableTests.serializable(Reducible[Id]))
}