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

Add combineAllOption to Foldable #2380

Merged
merged 11 commits into from
Nov 14, 2019
22 changes: 22 additions & 0 deletions core/src/main/scala-2.12/cats/compat/FoldableCompat.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package cats
package compat

private[cats] trait FoldableCompat[F[_]] { this: Foldable[F] =>
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is private[cats] can people override outside of cats? I don't know how this works when it is mixed into Foldble. Is it even publicly visible at all?

Copy link
Contributor Author

@barambani barambani Nov 9, 2019

Choose a reason for hiding this comment

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

I think this is not accessible outside cats so FoldableCompat cannot be extended but iterable can be overridden through Foldable.
I ran a quick test for this extending from outside the cats package with

object test {

  trait T1[F[_]] extends cats.Foldable[F] {
    override def iterable[A](fa: F[A]): Stream[A] = ???
  }

  trait T2[F[_]] extends cats.compat.FoldableCompat[F] {
    override def iterable[A](fa: F[A]): Stream[A] = ???
  }
}

and it seems that T1 is ok and T2 fails as expected on 2.12 and 2.13

[error] /Users/fmariotti/open-source/cats/core/src/main/scala/test.scala:7:38: trait FoldableCompat in package compat cannot be accessed in package cats.compat
[error]   trait T2[F[_]] extends cats.compat.FoldableCompat[F] {
[error]                                      ^
[error] one error found
[error] (coreJVM / Compile / compileIncremental) Compilation failed


def iterable[A](fa: F[A]): Stream[A] =
foldRight[A, Stream[A]](fa, Eval.now(Stream.empty)) { (a, eb) =>
eb.map(Stream.cons(a, _))
}.value
}

private[cats] object FoldableCompat {

trait ToFoldableCompatOps {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't make simulacrum generate the syntax for this so I added it manually.

implicit final def foldableCompatSyntax[F[_], A](fa: F[A]): FoldableCompatAllOps[F, A] =
new FoldableCompatAllOps(fa)
}

final class FoldableCompatAllOps[F[_], A](private val fa: F[A]) extends AnyVal {
def iterable(implicit F: Foldable[F]): Stream[A] = F.iterable(fa)
}
}
2 changes: 2 additions & 0 deletions core/src/main/scala-2.12/cats/instances/stream.scala
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ trait StreamInstances extends cats.kernel.instances.StreamInstances {

override def toList[A](fa: Stream[A]): List[A] = fa.toList

override def iterable[A](fa: Stream[A]): Stream[A] = fa

override def reduceLeftOption[A](fa: Stream[A])(f: (A, A) => A): Option[A] =
fa.reduceLeftOption(f)

Expand Down
22 changes: 22 additions & 0 deletions core/src/main/scala-2.13+/cats/compat/FoldableCompat.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package cats
package compat

private[cats] trait FoldableCompat[F[_]] { this: Foldable[F] =>

def iterable[A](fa: F[A]): LazyList[A] =
foldRight[A, LazyList[A]](fa, Eval.now(LazyList.empty)) { (a, eb) =>
eb.map(LazyList.cons(a, _))
}.value
}

private[cats] object FoldableCompat {

trait ToFoldableCompatOps {
implicit final def foldableCompatSyntax[F[_], A](fa: F[A]): FoldableCompatAllOps[F, A] =
new FoldableCompatAllOps(fa)
}

final class FoldableCompatAllOps[F[_], A](private val fa: F[A]) extends AnyVal {
def iterable(implicit F: Foldable[F]): LazyList[A] = F.iterable(fa)
}
}
2 changes: 2 additions & 0 deletions core/src/main/scala-2.13+/cats/instances/lazyList.scala
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ trait LazyListInstances extends cats.kernel.instances.LazyListInstances {

override def toList[A](fa: LazyList[A]): List[A] = fa.toList

override def iterable[A](fa: LazyList[A]): LazyList[A] = fa

override def reduceLeftOption[A](fa: LazyList[A])(f: (A, A) => A): Option[A] =
fa.reduceLeftOption(f)

Expand Down
2 changes: 2 additions & 0 deletions core/src/main/scala-2.13+/cats/instances/stream.scala
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ trait StreamInstances extends cats.kernel.instances.StreamInstances {

override def toList[A](fa: Stream[A]): List[A] = fa.toList

override def iterable[A](fa: Stream[A]): LazyList[A] = LazyList.from(fa)

override def reduceLeftOption[A](fa: Stream[A])(f: (A, A) => A): Option[A] =
fa.reduceLeftOption(f)

Expand Down
5 changes: 4 additions & 1 deletion core/src/main/scala/cats/Foldable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import Foldable.sentinel
*
* See: [[http://www.cs.nott.ac.uk/~pszgmh/fold.pdf A tutorial on the universality and expressiveness of fold]]
*/
@typeclass trait Foldable[F[_]] extends UnorderedFoldable[F] { self =>
@typeclass trait Foldable[F[_]] extends cats.compat.FoldableCompat[F] with UnorderedFoldable[F] { self =>

/**
* Left associative fold on 'F' using the function 'f'.
Expand Down Expand Up @@ -275,6 +275,9 @@ import Foldable.sentinel
*/
def combineAll[A: Monoid](fa: F[A]): A = fold(fa)

def combineAllOption[A](fa: F[A])(implicit ev: Semigroup[A]): Option[A] =
if (isEmpty(fa)) None else ev.combineAllOption(iterable(fa))
Copy link
Contributor

Choose a reason for hiding this comment

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

If we have iterable now, should we use it in fold for the default implementation? I think we should.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, it could be implemented in terms of the Monoid's combineAll like

A.combineAll(iterable(fa))

this would allow to optimize the fold providing a specialization for Monoid[A].
I'll add it in.


/**
* Fold implemented by mapping `A` values into `B` and then
* combining them using the given `Monoid[B]` instance.
Expand Down
6 changes: 5 additions & 1 deletion core/src/main/scala/cats/syntax/foldable.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package cats
package syntax

trait FoldableSyntax extends Foldable.ToFoldableOps with UnorderedFoldable.ToUnorderedFoldableOps {
trait FoldableSyntax
extends Foldable.ToFoldableOps
with cats.compat.FoldableCompat.ToFoldableCompatOps
with UnorderedFoldable.ToUnorderedFoldableOps {

implicit final def catsSyntaxNestedFoldable[F[_]: Foldable, G[_], A](fga: F[G[A]]): NestedFoldableOps[F, G, A] =
new NestedFoldableOps[F, G, A](fga)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class NonEmptyStreamSuite extends CatsSuite {
}

class ReducibleNonEmptyStreamSuite extends ReducibleSuite[NonEmptyStream]("NonEmptyStream") {
def iterator[T](nes: NonEmptyStream[T]): Iterator[T] =
override def iterator[T](nes: NonEmptyStream[T]): Iterator[T] =
(nes.head #:: nes.tail).iterator

def range(start: Long, endInclusive: Long): NonEmptyStream[Long] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ class NonEmptyLazyListSuite extends CatsSuite {
}

class ReducibleNonEmptyLazyListSuite extends ReducibleSuite[NonEmptyLazyList]("NonEmptyLazyList") {
def iterator[T](nel: NonEmptyLazyList[T]): Iterator[T] = nel.toLazyList.iterator
override def iterator[T](nel: NonEmptyLazyList[T]): Iterator[T] = nel.toLazyList.iterator

def range(start: Long, endInclusive: Long): NonEmptyLazyList[Long] =
NonEmptyLazyList(start, (start + 1L).to(endInclusive): _*)
Expand Down
13 changes: 13 additions & 0 deletions tests/src/test/scala/cats/tests/FoldableSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,19 @@ abstract class FoldableSuite[F[_]: Foldable](name: String)(implicit ArbFInt: Arb
}
}

test(s"Foldable[$name].combineAllOption") {
forAll { (fa: F[Int]) =>
val list = fa.toList
fa.combineAllOption should ===(list.combineAllOption)
}
}

test(s"Foldable[$name].iterable") {
forAll { (fa: F[Int]) =>
fa.iterable.toList should ===(iterator(fa).toList)
}
}

test(s"Foldable[$name].intercalate") {
forAll { (fa: F[String], a: String) =>
fa.intercalate(a) should ===(fa.toList.mkString(a))
Expand Down