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

Adding partitionEitherM #2641

Merged
merged 18 commits into from
Jan 22, 2019
1 change: 1 addition & 0 deletions core/src/main/scala/cats/syntax/all.scala
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,5 @@ trait AllSyntaxBinCompat4
with ParallelApplySyntax
with FoldableSyntaxBinCompat0
with ReducibleSyntaxBinCompat0
with FoldableSyntaxBinCompat1
with BitraverseSyntaxBinCompat0
147 changes: 146 additions & 1 deletion core/src/main/scala/cats/syntax/foldable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ trait FoldableSyntaxBinCompat0 {
new FoldableOps0[F, A](fa)
}

trait FoldableSyntaxBinCompat1 {
implicit final def catsSyntaxFoldableBinCompat0[F[_]](fa: Foldable[F]): FoldableOps1[F] =
new FoldableOps1(fa)
}

final class NestedFoldableOps[F[_], G[_], A](private val fga: F[G[A]]) extends AnyVal {
def sequence_(implicit F: Foldable[F], G: Applicative[G]): G[Unit] = F.sequence_(fga)

Expand Down Expand Up @@ -195,7 +200,6 @@ final class FoldableOps[F[_], A](private val fa: F[A]) extends AnyVal {
case None ⇒ acc
}
)

}

final class FoldableOps0[F[_], A](val fa: F[A]) extends AnyVal {
Expand All @@ -214,4 +218,145 @@ final class FoldableOps0[F[_], A](val fa: F[A]) extends AnyVal {
* */
def foldMapK[G[_], B](f: A => G[B])(implicit F: Foldable[F], G: MonoidK[G]): G[B] =
F.foldMap(fa)(f)(G.algebra)

/**
* Separate this Foldable into a Tuple by an effectful separating function `A => H[B, C]` for some `Bifoldable[H]`
* Equivalent to `Functor#map` over `Alternative#separate`
*
* {{{
* scala> import cats.implicits._, cats.data.Const
* scala> val list = List(1,2,3,4)
* scala> list.partitionBifold(a => (a, s"value ${a}"))
* res0: (List[Int], List[String]) = (List(1, 2, 3, 4),List(value 1, value 2, value 3, value 4))
* `Const`'s second parameter is never instantiated, so we can use an impossible type:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should line 231 have a // at the beginning of the comment? I guess that I don't know whether or not that's necessary in a doctest example; just wanted to make sure that this code is running when tests are run.

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 just assumed the parser rules were similar to literate extensions for languages. Altering line 232 to have a syntax error caused coreJVM/test:compile to fail to compile, which was good as it seems as though there are other instances of this: https://github.com/typelevel/cats/blob/774fb51/core/src/main/scala/cats/Foldable.scala#L48

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 should say, I'm not opposed to changing it, just that it currently isn't causing the tool to stop compiling the other lines.

* scala> list.partitionBifold(a => Const[Int, Nothing with Any](a))
* res1: (List[Int], List[Nothing with Any]) = (List(1, 2, 3, 4), List())
* }}}
*/
def partitionBifold[H[_, _], B, C](
f: A => H[B, C]
)(implicit A: Alternative[F], F: Foldable[F], H: Bifoldable[H]): (F[B], F[C]) = {
import cats.syntax.foldable._
F.partitionBifold[H, A, B, C](fa)(f)(A, H)
}

/**
* Separate this Foldable into a Tuple by an effectful separating function `A => G[H[B, C]]` for some `Bifoldable[H]`
* Equivalent to `Traverse#traverse` over `Alternative#separate`
*
* {{{
* scala> import cats.implicits._, cats.data.Const
* scala> val list = List(1,2,3,4)
* `Const`'s second parameter is never instantiated, so we can use an impossible type:
* scala> list.partitionBifoldM(a => Option(Const[Int, Nothing with Any](a)))
* res0: Option[(List[Int], List[Nothing with Any])] = Some((List(1, 2, 3, 4), List()))
* }}}
*/
def partitionBifoldM[G[_], H[_, _], B, C](
f: A => G[H[B, C]]
)(implicit A: Alternative[F], F: Foldable[F], M: Monad[G], H: Bifoldable[H]): G[(F[B], F[C])] = {
import cats.syntax.foldable._
F.partitionBifoldM[G, H, A, B, C](fa)(f)(A, M, H)
}

/**
* Separate this Foldable into a Tuple by an effectful separating function `A => G[Either[B, C]]`
* Equivalent to `Traverse#traverse` over `Alternative#separate`
*
* {{{
* scala> import cats.implicits._, cats.Eval
* scala> val list = List(1,2,3,4)
* scala> val partitioned1 = list.partitionEitherM(a => if (a % 2 == 0) Eval.now(Either.left[String, Int](a.toString)) else Eval.now(Either.right[String, Int](a)))
* Since `Eval.now` yields a lazy computation, we need to force it to inspect the result:
* scala> partitioned1.value
* res0: (List[String], List[Int]) = (List(2, 4),List(1, 3))
* scala> val partitioned2 = list.partitionEitherM(a => Eval.later(Either.right(a * 4)))
* scala> partitioned2.value
* res1: (List[Nothing], List[Int]) = (List(),List(4, 8, 12, 16))
* }}}
*/
def partitionEitherM[G[_], B, C](
f: A => G[Either[B, C]]
)(implicit A: Alternative[F], F: Foldable[F], M: Monad[G]): G[(F[B], F[C])] = {
import cats.syntax.foldable._
F.partitionEitherM[G, A, B, C](fa)(f)(A, M)
}
}

final class FoldableOps1[F[_]](private val F: Foldable[F]) extends AnyVal {

/**
* Separate this Foldable into a Tuple by a separating function `A => H[B, C]` for some `Bifoldable[H]`
* Equivalent to `Functor#map` and then `Alternative#separate`.
*
* {{{
* scala> import cats.implicits._, cats.Foldable, cats.data.Const
* scala> val list = List(1,2,3,4)
* scala> Foldable[List].partitionBifold(list)(a => (s"value ${a}", if (a % 2 == 0) -a else a))
* res0: (List[String], List[Int]) = (List(value 1, value 2, value 3, value 4),List(1, -2, 3, -4))
* scala> Foldable[List].partitionBifold(list)(a => Const[Int, Nothing with Any](a))
* res1: (List[Int], List[Nothing with Any]) = (List(1, 2, 3, 4),List())
* }}}
*/
def partitionBifold[H[_, _], A, B, C](fa: F[A])(f: A => H[B, C])(implicit A: Alternative[F],
H: Bifoldable[H]): (F[B], F[C]) = {
import cats.instances.tuple._

implicit val mb: Monoid[F[B]] = A.algebra[B]
implicit val mc: Monoid[F[C]] = A.algebra[C]

F.foldMap[A, (F[B], F[C])](fa)(
a => H.bifoldMap[B, C, (F[B], F[C])](f(a))(b => (A.pure(b), A.empty[C]), c => (A.empty[B], A.pure(c)))
)
}

/**
* Separate this Foldable into a Tuple by an effectful separating function `A => G[H[B, C]]` for some `Bifoldable[H]`
* Equivalent to `Traverse#traverse` over `Alternative#separate`
*
* {{{
* scala> import cats.implicits._, cats.Foldable, cats.data.Const
* scala> val list = List(1,2,3,4)
* `Const`'s second parameter is never instantiated, so we can use an impossible type:
* scala> Foldable[List].partitionBifoldM(list)(a => Option(Const[Int, Nothing with Any](a)))
* res0: Option[(List[Int], List[Nothing with Any])] = Some((List(1, 2, 3, 4), List()))
* }}}
*/
def partitionBifoldM[G[_], H[_, _], A, B, C](
fa: F[A]
)(f: A => G[H[B, C]])(implicit A: Alternative[F], M: Monad[G], H: Bifoldable[H]): G[(F[B], F[C])] = {
import cats.instances.tuple._

implicit val mb: Monoid[F[B]] = A.algebra[B]
implicit val mc: Monoid[F[C]] = A.algebra[C]

F.foldMapM[G, A, (F[B], F[C])](fa)(
a =>
M.map(f(a)) {
H.bifoldMap[B, C, (F[B], F[C])](_)(b => (A.pure(b), A.empty[C]), c => (A.empty[B], A.pure(c)))
}
)
}

/**
* Separate this Foldable into a Tuple by an effectful separating function `A => G[Either[B, C]]`
* Equivalent to `Traverse#traverse` over `Alternative#separate`
*
* {{{
* scala> import cats.implicits._, cats.Foldable, cats.Eval
* scala> val list = List(1,2,3,4)
* scala> val partitioned1 = Foldable[List].partitionEitherM(list)(a => if (a % 2 == 0) Eval.now(Either.left[String, Int](a.toString)) else Eval.now(Either.right[String, Int](a)))
* Since `Eval.now` yields a lazy computation, we need to force it to inspect the result:
* scala> partitioned1.value
* res0: (List[String], List[Int]) = (List(2, 4),List(1, 3))
* scala> val partitioned2 = Foldable[List].partitionEitherM(list)(a => Eval.later(Either.right(a * 4)))
* scala> partitioned2.value
* res1: (List[Nothing], List[Int]) = (List(),List(4, 8, 12, 16))
* }}}
*/
def partitionEitherM[G[_], A, B, C](fa: F[A])(f: A => G[Either[B, C]])(implicit A: Alternative[F],
Copy link
Contributor

Choose a reason for hiding this comment

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

This code is duplicated in two places. What if instead we added a partitionEitherM method to the Foldable companion object and had the FoldableOps0 method delegate to it? At that point, I'm not sure if it makes sense to have the FoldableOps1 extension to the type class itself; people could just call the method on the companion object if they wanted it. WDYT @kailuowang and @blast-hardcheese?

Sorry I'm sure that this is annoying. I think that Cats maintainers need to put some effort into our approach to binary compatibility and making it easier on other contributors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consolidating definitely felt like the right approach, but doing the same syntax lookup again while we're already working around bin compat seemed worse. I can make the change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to generalize this to any Bifoldable instead of just Either?

  def partitionBiffyM[G[_], H[_,_], A, B, C](fa: F[A])(f: A => G[H[B, C]])(implicit A: Alternative[F],
                                                                         M: Monad[G], H: Bifoldable[H]): G[(F[B], F[C])] = {
    import cats.instances.tuple._

    implicit val mb: Monoid[F[B]] = A.algebra[B]
    implicit val mc: Monoid[F[C]] = A.algebra[C]

    F.foldMapM[G, A, (F[B], F[C])](fa)(
      a =>
        M.map(f(a)){
          H.bifoldMap[B, C, (F[B], F[C])](_)(
            b => (A.pure(b), A.empty[C]),
            c => (A.empty[B], A.pure(c)))
      }
    )
  }

Then you could use it for the partitionEitherM use-case, but you could also use it for other Bifoldables like Tuple2:

scala> Foldable[List].partitionBiffyM(list)(a => Eval.now((a - 1, a + 1)))
res5: cats.Eval[(List[Int], List[Int])] = cats.Eval$$anon$6@67bef13e

scala> res5.value
res6: (List[Int], List[Int]) = (List(0, 1, 2, 3),List(2, 3, 4, 5))

I'm hoping that someone else has a better name than mine 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems to suggest a broader refactor that includes partitionEither. I can generalize to partitionBiffy as well as partitionBiffyM, while providing the concretized partitionEitherM that just defers to partitionBiffyM, if that is desirable

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 gave it a shot, though the examples are getting increasingly convoluted I think.

M: Monad[G]): G[(F[B], F[C])] = {
import cats.instances.either._
partitionBifoldM[G, Either, A, B, C](fa)(f)(A, M, Bifoldable[Either])
}
}
2 changes: 1 addition & 1 deletion core/src/main/scala/cats/syntax/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ package object syntax {
object either extends EitherSyntax with EitherSyntaxBinCompat0
object eq extends EqSyntax
object flatMap extends FlatMapSyntax
object foldable extends FoldableSyntax with FoldableSyntaxBinCompat0
object foldable extends FoldableSyntax with FoldableSyntaxBinCompat0 with FoldableSyntaxBinCompat1
object functor extends FunctorSyntax
object functorFilter extends FunctorFilterSyntax
object group extends GroupSyntax
Expand Down
57 changes: 57 additions & 0 deletions tests/src/test/scala/cats/tests/FoldableSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,63 @@ abstract class FoldableSuite[F[_]: Foldable](name: String)(implicit ArbFInt: Arb
}
}

test("Foldable#partitionEitherM retains size") {
import cats.syntax.foldable._
forAll { (fi: F[Int], f: Int => Either[String, String]) =>
val vector = Foldable[F].toList(fi).toVector
val result = Foldable[Vector].partitionEitherM(vector)(f.andThen(Option.apply)).map {
case (lefts, rights) =>
(lefts <+> rights).size
}
result should ===(Option(vector.size))
}
}

test("Foldable#partitionEitherM consistent with List#partition") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh this is a good idea for a test 👍

import cats.syntax.foldable._
forAll { (fi: F[Int], f: Int => Either[String, String]) =>
val list = Foldable[F].toList(fi)
val partitioned = Foldable[List].partitionEitherM(list)(f.andThen(Option.apply))
val (ls, rs) = list
.map(f)
.partition({
case Left(_) => true
case Right(_) => false
})

partitioned.map(_._1.map(_.asLeft[String])) should ===(Option(ls))
partitioned.map(_._2.map(_.asRight[String])) should ===(Option(rs))
}
}

test("Foldable#partitionEitherM to one side is identity") {
import cats.syntax.foldable._
forAll { (fi: F[Int], f: Int => String) =>
val list = Foldable[F].toList(fi)
val g: Int => Option[Either[Double, String]] = f.andThen(Right.apply).andThen(Option.apply)
val h: Int => Option[Either[String, Double]] = f.andThen(Left.apply).andThen(Option.apply)

val withG = Foldable[List].partitionEitherM(list)(g).map(_._2)
withG should ===(Option(list.map(f)))

val withH = Foldable[List].partitionEitherM(list)(h).map(_._1)
withH should ===(Option(list.map(f)))
}
}

test("Foldable#partitionEitherM remains sorted") {
import cats.syntax.foldable._
forAll { (fi: F[Int], f: Int => Either[String, String]) =>
val list = Foldable[F].toList(fi)

val sorted = list.map(f).sorted
val pairs = Foldable[List].partitionEitherM(sorted)(Option.apply)

pairs.map(_._1.sorted) should ===(pairs.map(_._1))
pairs.map(_._2.sorted) should ===(pairs.map(_._2))
}
}

test(s"Foldable[$name] summation") {
forAll { (fa: F[Int]) =>
val total = iterator(fa).sum
Expand Down