-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Adding partitionEitherM #2641
Changes from 6 commits
305b40e
6c1afdc
f36ce95
959a793
a23b71d
a522b0f
f394412
0912a58
6dfb3b4
79b7c6f
3cb7be1
cc7275d
b3fb6d3
95ab386
7f9923f
bd8f0cb
f4eef23
43e4276
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
|
||
|
@@ -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 { | ||
|
@@ -214,4 +218,62 @@ 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 => G[Either[B, C]]` | ||
* Equivalent to `Bitraversable#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 an effectful separating function `A => G[Either[B, C]]` | ||
* Equivalent to `Bitraversable#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], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to generalize this to any 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
I'm hoping that someone else has a better name than mine 😛 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to suggest a broader refactor that includes There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.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)) { | ||
case Right(c) => (A.empty[B], A.pure(c)) | ||
case Left(b) => (A.pure(b), A.empty[C]) | ||
} | ||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
There was a problem hiding this comment.
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 theFoldable
companion object and had theFoldableOps0
method delegate to it? At that point, I'm not sure if it makes sense to have theFoldableOps1
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.
There was a problem hiding this comment.
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.