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 NonEmptyList#partitionE #1858

Merged
merged 18 commits into from
Sep 6, 2017
25 changes: 25 additions & 0 deletions core/src/main/scala/cats/Foldable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,31 @@ import simulacrum.typeclass
buf += a
}.toList

/**
* Separate this Foldable into a Tuple by a separating function `A => Either[B, C]`
* Equivalent to `Functor#map` and then `Alternative#separate`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we did define it in Alternative, we could add a consistency law checking this equality, but hey ... you can't always get what you want 😃

Copy link
Member Author

Choose a reason for hiding this comment

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

True, I don't really mind either way tbh.
We could add a test that checks if it's consistent with List#partition though 😄

*
* {{{
* scala> import cats.implicits._
* scala> val list = List(1,2,3,4)
* scala> Foldable[List].partitionEither(list)(a => if (a % 2 == 0) Left(a.toString) else Right(a))
* res0: (List[String], List[Int]) = (List(2, 4),List(1, 3))
* scala> Foldable[List].partitionEither(list)(a => Right(a * 4))
* res1: (List[Nothing], List[Int]) = (List(),List(4, 8, 12, 16))
* }}}
*/
def partitionEither[A, B, C](fa: F[A])(f: A => Either[B, C])(implicit A: Alternative[F]): (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]

foldMap(fa)(a => f(a) match {
case Right(c) => (A.empty[B], A.pure(c))
case Left(b) => (A.pure(b), A.empty[C])
})
}

/**
* Convert F[A] to a List[A], only including elements which match `p`.
*/
Expand Down
31 changes: 29 additions & 2 deletions core/src/main/scala/cats/Reducible.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package cats

import cats.data.NonEmptyList

import cats.data.{Ior, NonEmptyList}
import simulacrum.typeclass

/**
Expand Down Expand Up @@ -177,6 +176,34 @@ import simulacrum.typeclass
Reducible[NonEmptyList].reduce(NonEmptyList(hd, a :: intersperseList(tl, a)))
}

/**
* Partition this Reducible by a separating function `A => Either[B, C]`
*
* {{{
* scala> import cats.data.NonEmptyList
* scala> val nel = NonEmptyList.of(1,2,3,4)
* scala> Reducible[NonEmptyList].nonEmptyPartition(nel)(a => if (a % 2 == 0) Left(a.toString) else Right(a))
* res0: cats.data.Ior[cats.data.NonEmptyList[String],cats.data.NonEmptyList[Int]] = Both(NonEmptyList(2, 4),NonEmptyList(1, 3))
* scala> Reducible[NonEmptyList].nonEmptyPartition(nel)(a => Right(a * 4))
* res1: cats.data.Ior[cats.data.NonEmptyList[Nothing],cats.data.NonEmptyList[Int]] = Right(NonEmptyList(4, 8, 12, 16))
* }}}
*/
def nonEmptyPartition[A, B, C](fa: F[A])(f: A => Either[B, C]): Ior[NonEmptyList[B], NonEmptyList[C]] = {
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 we do this considerably more efficiently for NonEmptyList and NonEmptyVector? Can we add methods to those datatypes that does so? Eval is kind of a perf killer, sadly, which this method hides.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just had the same idea :D I already implemented a O(n) solution for partitionEither on List and will override nonemptyPartition as well!

Copy link
Member Author

Choose a reason for hiding this comment

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

Added overrides for NEL and NEV :)

import cats.syntax.either._

def g(a: A, eval: Eval[Ior[NonEmptyList[B], NonEmptyList[C]]]): Eval[Ior[NonEmptyList[B], NonEmptyList[C]]] = {
eval.map(ior =>
(f(a), ior) match {
case (Right(c), Ior.Left(_)) => ior.putRight(NonEmptyList.one(c))
case (Right(c), _) => ior.map(c :: _)
case (Left(b), Ior.Right(r)) => Ior.bothNel(b, r)
case (Left(b), _) => ior.leftMap(b :: _)
})
}

reduceRightTo(fa)(a => f(a).bimap(NonEmptyList.one, NonEmptyList.one).toIor)(g).value
}

override def isEmpty[A](fa: F[A]): Boolean = false

override def nonEmpty[A](fa: F[A]): Boolean = true
Expand Down
17 changes: 17 additions & 0 deletions core/src/main/scala/cats/data/NonEmptyList.scala
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,23 @@ private[data] sealed trait NonEmptyListInstances extends NonEmptyListInstances0
override def fold[A](fa: NonEmptyList[A])(implicit A: Monoid[A]): A =
fa.reduce

override def nonEmptyPartition[A, B, C](fa: NonEmptyList[A])
(f: (A) => Either[B, C]): Ior[NonEmptyList[B], NonEmptyList[C]] = {
import cats.syntax.either._

val reversed = fa.reverse
val lastIor = f(reversed.head).bimap(NonEmptyList.one, NonEmptyList.one).toIor

reversed.tail.foldLeft(lastIor)((ior, a) => (f(a), ior) match {
case (Right(c), Ior.Left(_)) => ior.putRight(NonEmptyList.one(c))
case (Right(c), _) => ior.map(c :: _)
case (Left(b), Ior.Right(r)) => Ior.bothNel(b, r)
case (Left(b), _) => ior.leftMap(b :: _)
})

}


override def find[A](fa: NonEmptyList[A])(f: A => Boolean): Option[A] =
fa find f

Expand Down
15 changes: 14 additions & 1 deletion core/src/main/scala/cats/data/NonEmptyVector.scala
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,19 @@ private[data] sealed trait NonEmptyVectorInstances {
override def foldRight[A, B](fa: NonEmptyVector[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] =
fa.foldRight(lb)(f)

override def nonEmptyPartition[A, B, C](fa: NonEmptyVector[A])(f: (A) => Either[B, C]): Ior[NonEmptyList[B], NonEmptyList[C]] = {
import cats.syntax.either._
import cats.syntax.reducible._

reduceLeftTo(fa)(a => f(a).bimap(NonEmptyVector.one, NonEmptyVector.one).toIor)((ior, a) => (f(a), ior) match {
case (Right(c), Ior.Left(_)) => ior.putRight(NonEmptyVector.one(c))
case (Right(c), _) => ior.map(_ :+ c)
case (Left(b), Ior.Right(_)) => ior.putLeft(NonEmptyVector.one(b))
case (Left(b), _) => ior.leftMap(_ :+ b)
}).bimap(_.toNonEmptyList, _.toNonEmptyList)

}

override def get[A](fa: NonEmptyVector[A])(idx: Long): Option[A] =
if (0 <= idx && idx < Int.MaxValue) fa.get(idx.toInt) else None

Expand Down Expand Up @@ -306,7 +319,7 @@ private[data] sealed trait NonEmptyVectorInstances {

}

object NonEmptyVector extends NonEmptyVectorInstances {
object NonEmptyVector extends NonEmptyVectorInstances with Serializable {

def apply[A](head: A, tail: Vector[A]): NonEmptyVector[A] =
new NonEmptyVector(head +: tail)
Expand Down
8 changes: 8 additions & 0 deletions core/src/main/scala/cats/instances/list.scala
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ trait ListInstances extends cats.kernel.instances.ListInstances {
override def zipWithIndex[A](fa: List[A]): List[(A, Int)] =
fa.zipWithIndex

override def partitionEither[A, B, C](fa: List[A])
(f: (A) => Either[B, C])
(implicit A: Alternative[List]): (List[B], List[C]) =
fa.foldRight((List.empty[B], List.empty[C]))((a, acc) => f(a) match {
case Left(b) => (b :: acc._1, acc._2)
case Right(c) => (acc._1, c :: acc._2)
})

@tailrec
override def get[A](fa: List[A])(idx: Long): Option[A] =
fa match {
Expand Down
48 changes: 48 additions & 0 deletions tests/src/test/scala/cats/tests/FoldableTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,54 @@ abstract class FoldableCheck[F[_]: Foldable](name: String)(implicit ArbFInt: Arb
}
}

test("Foldable#partitionEither retains size") {
forAll { (fi: F[Int], f: Int => Either[String, String]) =>
val vector = Foldable[F].toList(fi).toVector
val (lefts, rights) = Foldable[Vector].partitionEither(vector)(f)
(lefts <+> rights).size.toLong should === (fi.size)
}
}

test("Foldable#partitionEither consistent with List#partition") {
forAll { (fi: F[Int], f: Int => Either[String, String]) =>
val list = Foldable[F].toList(fi)
val (lefts, rights) = Foldable[List].partitionEither(list)(f)
val (ls, rs) = list.map(f).partition({
case Left(_) => true
case Right(_) => false
})

lefts.map(_.asLeft[String]) should === (ls)
rights.map(_.asRight[String]) should === (rs)
}
}

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

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

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

test("Foldable#partitionEither remains sorted") {
forAll { (fi: F[Int], f: Int => Either[String, String]) =>
val list = Foldable[F].toList(fi)

val sorted = list.map(f).sorted
val (lefts, rights) = Foldable[List].partitionEither(sorted)(identity)

lefts.sorted should === (lefts)
rights.sorted should === (rights)
}
}

test(s"Foldable[$name] summation") {
forAll { (fa: F[Int]) =>
val total = iterator(fa).sum
Expand Down
14 changes: 13 additions & 1 deletion tests/src/test/scala/cats/tests/NonEmptyListTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class NonEmptyListTests extends CatsSuite {
}
}


test("NonEmptyList#filter is consistent with List#filter") {
forAll { (nel: NonEmptyList[Int], p: Int => Boolean) =>
val list = nel.toList
Expand Down Expand Up @@ -281,9 +282,20 @@ class NonEmptyListTests extends CatsSuite {
}
}


test("NonEmptyList#zipWith is consistent with List#zip and then List#map") {
forAll { (a: NonEmptyList[Int], b: NonEmptyList[Int], f: (Int, Int) => Int) =>
a.zipWith(b)(f).toList should === (a.toList.zip(b.toList).map {case (x, y) => f(x, y)})
a.zipWith(b)(f).toList should ===(a.toList.zip(b.toList).map { case (x, y) => f(x, y) })
}
}
test("NonEmptyList#nonEmptyPartition remains sorted") {
forAll { (nel: NonEmptyList[Int], f: Int => Either[String, String]) =>

val sorted = nel.map(f).sorted
val ior = Reducible[NonEmptyList].nonEmptyPartition(sorted)(identity)

ior.left.map(xs => xs.sorted should === (xs))
ior.right.map(xs => xs.sorted should === (xs))
}
}
}
Expand Down
13 changes: 12 additions & 1 deletion tests/src/test/scala/cats/tests/NonEmptyVectorTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,20 @@ class NonEmptyVectorTests extends CatsSuite {
}
}


test("NonEmptyVector#zipWith is consistent with Vector#zip and then Vector#map") {
forAll { (a: NonEmptyVector[Int], b: NonEmptyVector[Int], f: (Int, Int) => Int) =>
a.zipWith(b)(f).toVector should === (a.toVector.zip(b.toVector).map { case (x, y) => f(x, y)})
a.zipWith(b)(f).toVector should ===(a.toVector.zip(b.toVector).map { case (x, y) => f(x, y) })
}
}
test("NonEmptyVector#nonEmptyPartition remains sorted") {
forAll { (nev: NonEmptyVector[Int], f: Int => Either[String, String]) =>

val sorted = NonEmptyVector.fromVectorUnsafe(nev.map(f).toVector.sorted)
val ior = Reducible[NonEmptyVector].nonEmptyPartition(sorted)(identity)

ior.left.map(xs => xs.sorted should === (xs))
ior.right.map(xs => xs.sorted should === (xs))
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions tests/src/test/scala/cats/tests/OneAndTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,19 @@ class OneAndTests extends CatsSuite {
}
}

test("NonEmptyStream#nonEmptyPartition remains sorted") {
forAll { (nes: NonEmptyStream[Int], f: Int => Either[String, String]) =>

val nesf = nes.map(f)
val sortedStream = (nesf.head #:: nesf.tail).sorted
val sortedNes = OneAnd(sortedStream.head, sortedStream.tail)
val ior = Reducible[NonEmptyStream].nonEmptyPartition(sortedNes)(identity)

ior.left.map(xs => xs.sorted should === (xs))
ior.right.map(xs => xs.sorted should === (xs))
}
}

test("reduceLeft consistent with foldLeft") {
forAll { (nel: NonEmptyStream[Int], f: (Int, Int) => Int) =>
nel.reduceLeft(f) should === (nel.tail.foldLeft(nel.head)(f))
Expand Down
22 changes: 22 additions & 0 deletions tests/src/test/scala/cats/tests/ReducibleTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,26 @@ abstract class ReducibleCheck[F[_]: Reducible](name: String)(implicit ArbFInt: A
fa.nonEmptyIntercalate(a) === (fa.toList.mkString(a))
}
}


test("Reducible#nonEmptyPartition retains size") {
forAll { (fi: F[Int], f: Int => Either[String, String]) =>
val folded = fi.nonEmptyPartition(f).fold(identity, identity, _ ++ _.toList)
folded.size.toLong should === (fi.size)
}
}

test("Reducible#nonEmptyPartition to one side is identity") {
forAll { (fi: F[Int], f: Int => String) =>
val g: Int => Either[Double, String] = f andThen Right.apply
val h: Int => Either[String, Double] = f andThen Left.apply

val withG = fi.nonEmptyPartition(g).right.getOrElse(NonEmptyList.one(""))
withG should === (Reducible[F].toNonEmptyList(fi).map(f))

val withH = fi.nonEmptyPartition(h).left.getOrElse(NonEmptyList.one(""))
withH should === (Reducible[F].toNonEmptyList(fi).map(f))
}
}

}