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 FunctorFilter and TraverseFilter #2405

Merged
merged 15 commits into from
Sep 3, 2018
Merged
22 changes: 22 additions & 0 deletions core/src/main/scala/cats/FunctorEmpty.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package cats

import simulacrum.typeclass

/**
* `FunctorEmpty[F]` allows you to `map` and filter out elements simultaneously.
*/
@typeclass
trait FunctorEmpty[F[_]] extends Serializable {
def functor: Functor[F]

Copy link
Contributor

Choose a reason for hiding this comment

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

Why doesn’t this have an empty method with the law that anything filtered out becomes empty?

Also, if you have a FunctorEmpty and MonoidK empty should be consistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we define any extra methods if we require implementors to implement an empty method? Or just for the laws? The Haskell precedent doesn't seem to define a method like this and I'm not sure if we should either, need to think about what the actual benefits are 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

well, I guess if you have an empty, since you can't combine it, you are always stuck with an empty. But still, that's interesting for laws empty.map(fn) == empty etc...

I hate to have a hacky way to make an empty, but not provide it (namely give me any instance, and I will filter it).

Copy link
Member Author

Choose a reason for hiding this comment

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

Here's some discussion on it #1365 and here's the haskell counter part http://hackage.haskell.org/package/witherable-0.2/docs/Data-Witherable.html

Copy link
Member Author

Choose a reason for hiding this comment

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

The original PR also has some discussion on why it wasn't included in the first place https://github.com/typelevel/cats/pull/1225/files#discussion_r71987653

Copy link
Contributor

Choose a reason for hiding this comment

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

More specifically, this comment: #1225 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah after thinking about it more a bit, I think I'd vote to keep it as is without the empty method.

def mapFilter[A, B](fa: F[A])(f: A => Option[B]): F[B]

def collect[A, B](fa: F[A])(f: PartialFunction[A, B]): F[B] =
mapFilter(fa)(f.lift)

def flattenOption[A](fa: F[Option[A]]): F[A] =
mapFilter(fa)(identity)

def filter[A](fa: F[A])(f: A => Boolean): F[A] =
mapFilter(fa)(a => if (f(a)) Some(a) else None)
}
26 changes: 26 additions & 0 deletions core/src/main/scala/cats/TraverseEmpty.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package cats

import simulacrum.typeclass

/**
* `TraverseEmpty`, also known as `Witherable`, represents list-like structures
* that can essentially have a `traverse` and a `filter` applied as a single
* combined operation (`traverseFilter`).
*
* Based on Haskell's [[https://hackage.haskell.org/package/witherable-0.1.3.3/docs/Data-Witherable.html Data.Witherable]]
*/

@typeclass
trait TraverseEmpty[F[_]] extends FunctorEmpty[F] {
def traverse: Traverse[F]

override def functor: Functor[F] = traverse
Copy link
Member

Choose a reason for hiding this comment

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

I haven't spent a lot of time on the this type of encoding. Is there a reason that functor should ever not be the traverse? In other words, do we lose anything by making this final?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, I agree fully, I'll change it :)


def traverseFilter[G[_], A, B](fa: F[A])(f: A => G[Option[B]])(implicit G: Applicative[G]): G[F[B]]

def filterA[G[_], A](fa: F[A])(f: A => G[Boolean])(implicit G: Applicative[G]): G[F[A]] =
traverseFilter(fa)(a => G.map(f(a))(if (_) Some(a) else None))

override def mapFilter[A, B](fa: F[A])(f: A => Option[B]): F[B] =
traverseFilter[Id, A, B](fa)(f)
}
23 changes: 23 additions & 0 deletions core/src/main/scala/cats/data/Chain.scala
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,29 @@ private[data] sealed abstract class ChainInstances extends ChainInstances1 {
}
}

implicit val catsDataTraverseEmptyForChain: TraverseEmpty[Chain] = new TraverseEmpty[Chain] {
def traverse: Traverse[Chain] = Chain.catsDataInstancesForChain

override def filter[A](fa: Chain[A])(f: A => Boolean): Chain[A] = fa.filter(f)

override def collect[A, B](fa: Chain[A])(f: PartialFunction[A, B]): Chain[B] = fa.collect(f)

override def mapFilter[A, B](fa: Chain[A])(f: A => Option[B]): Chain[B] = fa.collect(Function.unlift(f))

override def flattenOption[A](fa: Chain[Option[A]]): Chain[A] = fa.collect { case Some(a) => a }

def traverseFilter[G[_], A, B](fa: Chain[A])(f: A => G[Option[B]])(implicit G: Applicative[G]): G[Chain[B]] =
fa.foldRight(G.pure(Chain.empty[B]))(
(a, gcb) => G.map2(f(a), gcb)((ob, cb) => ob.fold(cb)(_ +: cb))
)

override def filterA[G[_], A](fa: Chain[A])(f: A => G[Boolean])(implicit G: Applicative[G]): G[Chain[A]] =
fa.foldRight(G.pure(Chain.empty[A]))(
(a, gca) =>
G.map2(f(a), gca)((b, chain) => if (b) a +: chain else chain))

}

}

private[data] sealed abstract class ChainInstances1 extends ChainInstances2 {
Expand Down
19 changes: 19 additions & 0 deletions core/src/main/scala/cats/data/Const.scala
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,25 @@ private[data] sealed abstract class ConstInstances extends ConstInstances0 {
fa.traverse(f)
}

implicit def catsDataTraverseEmptyForConst[C]: TraverseEmpty[Const[C, ?]] = new TraverseEmpty[Const[C, ?]] {

override def mapFilter[A, B](fa: Const[C, A])(f: (A) => Option[B]): Const[C, B] = fa.retag

override def collect[A, B](fa: Const[C, A])(f: PartialFunction[A, B]): Const[C, B] = fa.retag

override def flattenOption[A](fa: Const[C, Option[A]]): Const[C, A] = fa.retag

override def filter[A](fa: Const[C, A])(f: (A) => Boolean): Const[C, A] = fa.retag

def traverseFilter[G[_], A, B](fa: Const[C, A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Const[C, B]] =
G.pure(fa.retag[B])

override def filterA[G[_], A](fa: Const[C, A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[Const[C, A]] =
G.pure(fa)

val traverse: Traverse[Const[C, ?]] = Const.catsDataTraverseForConst[C]
}

implicit def catsDataMonoidForConst[A: Monoid, B]: Monoid[Const[A, B]] = new Monoid[Const[A, B]]{
def empty: Const[A, B] =
Const.empty
Expand Down
47 changes: 47 additions & 0 deletions core/src/main/scala/cats/data/EitherT.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package data

import cats.Bifunctor
import cats.instances.either._
import cats.instances.option._
import cats.syntax.either._

/**
Expand Down Expand Up @@ -505,6 +506,28 @@ private[data] abstract class EitherTInstances extends EitherTInstances1 {
def defer[A](fa: => EitherT[F, L, A]): EitherT[F, L, A] =
EitherT(F.defer(fa.value))
}

implicit def catsDataTraverseEmptyForEitherT[F[_], L](implicit F0: TraverseEmpty[F]): TraverseEmpty[EitherT[F, L, ?]] =
new EitherTFunctorEmpty[F, L] with TraverseEmpty[EitherT[F, L, ?]] {
implicit def F: FunctorEmpty[F] = F0
def traverse: Traverse[EitherT[F, L, ?]] = catsDataTraverseForEitherT[F, L](F0.traverse)

def traverseFilter[G[_], A, B]
(fa: EitherT[F, L, A])
(f: A => G[Option[B]])
(implicit G: Applicative[G]): G[EitherT[F, L, B]] =
G.map(
F0.traverseFilter[G, Either[L, A], Either[L, B]](fa.value) {
case l@Left(_) => G.pure(Option(l.rightCast[B]))
Copy link
Contributor

Choose a reason for hiding this comment

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

The Option.apply call here is either doing an unnecessary null-check here or is masking a null. I think that we should just use the Some constructor instead (and I believe that we have followed this convention elsewhere in Cats).

case Right(a) => G.map(f(a))(_.map(Either.right))
})(EitherT(_))

override def filterA[G[_], A]
(fa: EitherT[F, L, A])
(f: A => G[Boolean])
(implicit G: Applicative[G]): G[EitherT[F, L, A]] =
G.map(F0.filterA(fa.value)(_.fold(_ => G.pure(true), f)))(EitherT[F, L, A])
Copy link
Contributor

Choose a reason for hiding this comment

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

Here and in the filter method, it isn't obvious to me that a Left should result in true as opposed to false. Is there a law that guides us in this direction? If not, was there a particular reason for this choice? I'm not saying that I think that it should be the other way around; just that I wouldn't have known which one to pick. And for the Nested instances, we use a Traverse for the outer type and a TraverseFilter for the inner type, so it seems a little weird to me that you couldn't get a TraverseFilter instance for an F[Either[L, ?]] but if you lift it into an EitherT you can get one. This makes me question the validity/consistency/intuitiveness of this instance.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just the implementation we had in cats-mtl, so I'm not sure what the motivation was. I agree that it seems weird though, maybe we should delete it. Did you provide one in your initial PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I didn't. I'd be inclined to leave it out until someone makes a good argument for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah agree, can always add it back later 👍

}
}

private[data] abstract class EitherTInstances1 extends EitherTInstances2 {
Expand Down Expand Up @@ -536,6 +559,9 @@ private[data] abstract class EitherTInstances1 extends EitherTInstances2 {
override def ensureOr[A](fa: EitherT[F, L, A])(error: (A) => L)(predicate: (A) => Boolean): EitherT[F, L, A] =
fa.ensureOr(error)(predicate)(F)
}

implicit def catsDataFunctorEmptyForEitherT[F[_], L](implicit F0: FunctorEmpty[F]): FunctorEmpty[EitherT[F, L, ?]] =
new EitherTFunctorEmpty[F, L] { implicit def F = F0 }
}

private[data] abstract class EitherTInstances2 extends EitherTInstances3 {
Expand Down Expand Up @@ -695,3 +721,24 @@ private[data] sealed trait EitherTOrder[F[_], L, A] extends Order[EitherT[F, L,

override def compare(x: EitherT[F, L, A], y: EitherT[F, L, A]): Int = x compare y
}

private[data] sealed trait EitherTFunctorEmpty[F[_], E] extends FunctorEmpty[EitherT[F, E, ?]] {
implicit def F: FunctorEmpty[F]

override def functor: Functor[EitherT[F, E, ?]] = EitherT.catsDataFunctorForEitherT[F, E](F.functor)

def mapFilter[A, B](fa: EitherT[F, E, A])(f: (A) => Option[B]): EitherT[F, E, B] =
EitherT[F, E, B](F.mapFilter(fa.value)(_.traverse(f)))

override def collect[A, B](fa: EitherT[F, E, A])(f: PartialFunction[A, B]): EitherT[F, E, B] = {
EitherT[F, E, B](F.mapFilter(fa.value)(_.traverse(f.lift)))
}

override def flattenOption[A](fa: EitherT[F, E, Option[A]]): EitherT[F, E, A] = {
EitherT[F, E, A](F.flattenOption[Either[E, A]](F.functor.map(fa.value)(Traverse[Either[E, ?]].sequence[Option, A])))
}

override def filter[A](fa: EitherT[F, E, A])(f: (A) => Boolean): EitherT[F, E, A] = {
EitherT[F, E, A](F.filter(fa.value)(_.forall(f)))
}
}
41 changes: 40 additions & 1 deletion core/src/main/scala/cats/data/Nested.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package cats
package data



/** Similar to [[cats.data.Tuple2K]], but for nested composition.
*
* For instance, since both `List` and `Option` have a `Functor`, then so does
Expand Down Expand Up @@ -54,6 +53,14 @@ private[data] sealed abstract class NestedInstances extends NestedInstances0 {
def defer[A](fa: => Nested[F, G, A]): Nested[F, G, A] =
Nested(F.defer(fa.value))
}

implicit def catsDataTraverseEmptyForNested[F[_], G[_]](implicit F0: Traverse[F], G0: TraverseEmpty[G]): TraverseEmpty[Nested[F, G, ?]] =
new NestedTraverseEmpty[F, G] {
implicit val F: Traverse[F] = F0
implicit val G: TraverseEmpty[G] = G0
}


Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: extra newline here.

}

private[data] sealed abstract class NestedInstances0 extends NestedInstances1 {
Expand Down Expand Up @@ -315,3 +322,35 @@ private[data] trait NestedInvariantSemigroupalApply[F[_], G[_]] extends Invarian
def product[A, B](fa: Nested[F, G, A], fb: Nested[F, G, B]): Nested[F, G, (A, B)] =
Nested(FG.product(fa.value, fb.value))
}

private[data] abstract class NestedTraverseEmpty[F[_], G[_]] extends TraverseEmpty[Nested[F, G, ?]] {
implicit val F: Traverse[F]

implicit val G: TraverseEmpty[G]

def traverse: Traverse[Nested[F, G, ?]] = Nested.catsDataTraverseForNested(F, G.traverse)

override def mapFilter[A, B](fa: Nested[F, G, A])(f: (A) => Option[B]): Nested[F, G, B] =
Nested[F, G, B](F.map(fa.value)(G.mapFilter(_)(f)))

override def collect[A, B](fa: Nested[F, G, A])(f: PartialFunction[A, B]): Nested[F, G, B] =
Nested[F, G, B](F.map(fa.value)(G.collect(_)(f)))

override def flattenOption[A](fa: Nested[F, G, Option[A]]): Nested[F, G, A] =
Nested[F, G, A](F.map(fa.value)(G.flattenOption))

override def filter[A](fa: Nested[F, G, A])(f: (A) => Boolean): Nested[F, G, A] =
Nested[F, G, A](F.map(fa.value)(G.filter(_)(f)))

override def filterA[H[_], A]
(fa: Nested[F, G, A])
(f: A => H[Boolean])
(implicit H: Applicative[H]): H[Nested[F, G, A]] =
H.map(F.traverse(fa.value)(G.filterA[H, A](_)(f)))(Nested[F, G, A])

def traverseFilter[H[_], A, B]
(fga: Nested[F, G, A])
(f: A => H[Option[B]])
(implicit H: Applicative[H]): H[Nested[F, G, B]] =
H.map(F.traverse[H, G[A], G[B]](fga.value)(ga => G.traverseFilter(ga)(f)))(Nested[F, G, B])
}
14 changes: 14 additions & 0 deletions core/src/main/scala/cats/data/OptionT.scala
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,20 @@ private[data] sealed abstract class OptionTInstances extends OptionTInstances0 {
def defer[A](fa: => OptionT[F, A]): OptionT[F, A] =
OptionT(F.defer(fa.value))
}

implicit def optionTFunctorEmpty[F[_]: Functor]: FunctorEmpty[OptionT[F, ?]] = {
new FunctorEmpty[OptionT[F, ?]] {
override val functor: Functor[OptionT[F, ?]] = OptionT.catsDataFunctorForOptionT[F]

override def mapFilter[A, B](fa: OptionT[F, A])(f: (A) => Option[B]): OptionT[F, B] = fa.subflatMap(f)

override def collect[A, B](fa: OptionT[F, A])(f: PartialFunction[A, B]): OptionT[F, B] = fa.subflatMap(f.lift)

override def flattenOption[A](fa: OptionT[F, Option[A]]): OptionT[F, A] = fa.subflatMap(identity)

override def filter[A](fa: OptionT[F, A])(f: (A) => Boolean): OptionT[F, A] = fa.filter(f)
}
}
}

private[data] sealed abstract class OptionTInstances0 extends OptionTInstances1 {
Expand Down
7 changes: 6 additions & 1 deletion core/src/main/scala/cats/instances/all.scala
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,9 @@ trait AllInstancesBinCompat0
with Tuple2InstancesBinCompat0

trait AllInstancesBinCompat1
extends MapInstancesBinCompat0
extends OptionInstancesBinCompat0
with ListInstancesBinCompat0
with VectorInstancesBinCompat0
with StreamInstancesBinCompat0
with MapInstancesBinCompat0
with SortedMapInstancesBinCompat0
26 changes: 26 additions & 0 deletions core/src/main/scala/cats/instances/list.scala
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,29 @@ trait ListInstances extends cats.kernel.instances.ListInstances {
fa.iterator.map(_.show).mkString("List(", ", ", ")")
}
}

trait ListInstancesBinCompat0 {
implicit val catsStdTraverseEmptyForList: TraverseEmpty[List] = new TraverseEmpty[List] {
val traverse: Traverse[List] = cats.instances.list.catsStdInstancesForList

override def mapFilter[A, B](fa: List[A])(f: (A) => Option[B]): List[B] = fa.collect(Function.unlift(f))

override def filter[A](fa: List[A])(f: (A) => Boolean): List[A] = fa.filter(f)

override def collect[A, B](fa: List[A])(f: PartialFunction[A, B]): List[B] = fa.collect(f)

override def flattenOption[A](fa: List[Option[A]]): List[A] = fa.flatten

def traverseFilter[G[_], A, B](fa: List[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[List[B]] =
fa.foldRight(Eval.now(G.pure(List.empty[B])))(
(x, xse) =>
G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ :: o))
).value

override def filterA[G[_], A](fa: List[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[List[A]] =
fa.foldRight(Eval.now(G.pure(List.empty[A])))(
(x, xse) =>
G.map2Eval(f(x), xse)((b, list) => if (b) x :: list else list)
).value
}
}
19 changes: 19 additions & 0 deletions core/src/main/scala/cats/instances/map.scala
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,26 @@ trait MapInstancesBinCompat0 {
}
}
}
}

implicit def catsStdFunctorEmptyForMap[K]: FunctorEmpty[Map[K, ?]] = {
new FunctorEmpty[Map[K, ?]] {

val functor: Functor[Map[K, ?]] = cats.instances.map.catsStdInstancesForMap[K]

def mapFilter[A, B](fa: Map[K, A])(f: A => Option[B]) =
fa.collect(scala.Function.unlift(t => f(t._2).map(t._1 -> _)))

override def collect[A, B](fa: Map[K, A])(f: PartialFunction[A, B]) =
fa.collect(scala.Function.unlift(t => f.lift(t._2).map(t._1 -> _)))

override def flattenOption[A](fa: Map[K, Option[A]]) =
fa.collect(scala.Function.unlift(t => t._2.map(t._1 -> _)))

override def filter[A](fa: Map[K, A])(f: A => Boolean) =
fa.filter { case (_, v) => f(v) }

}
}

}
27 changes: 27 additions & 0 deletions core/src/main/scala/cats/instances/option.scala
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,30 @@ trait OptionInstances extends cats.kernel.instances.OptionInstances {
}
}
}

trait OptionInstancesBinCompat0 {
implicit val catsStdTraverseEmptyForOption: TraverseEmpty[Option] = new TraverseEmpty[Option] {
val traverse: Traverse[Option] = cats.instances.option.catsStdInstancesForOption

override def mapFilter[A, B](fa: Option[A])(f: (A) => Option[B]): Option[B] = fa.flatMap(f)

override def filter[A](fa: Option[A])(f: (A) => Boolean): Option[A] = fa.filter(f)

override def collect[A, B](fa: Option[A])(f: PartialFunction[A, B]): Option[B] = fa.collect(f)

override def flattenOption[A](fa: Option[Option[A]]): Option[A] = fa.flatten

def traverseFilter[G[_], A, B](fa: Option[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Option[B]] =
fa match {
case _: None.type => G.pure(Option.empty[B])
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: I think that this could just be case None

case Some(a) => f(a)
}

override def filterA[G[_], A](fa: Option[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[Option[A]] =
fa match {
case _: None.type => G.pure(Option.empty[A])
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above

case Some(a) => G.map(f(a))(b => if (b) Some(a) else None)
}

}
}
14 changes: 6 additions & 8 deletions core/src/main/scala/cats/instances/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,14 @@ package object instances {
object eq extends EqInstances
object equiv extends EquivInstances
object float extends FloatInstances
object function extends FunctionInstances
with FunctionInstancesBinCompat0
object function extends FunctionInstances with FunctionInstancesBinCompat0
object future extends FutureInstances
object int extends IntInstances
object invariant extends InvariantMonoidalInstances
object list extends ListInstances
object list extends ListInstances with ListInstancesBinCompat0
object long extends LongInstances
object option extends OptionInstances with OptionInstancesBinCompat0
object map extends MapInstances with MapInstancesBinCompat0
object option extends OptionInstances
object order extends OrderInstances
object ordering extends OrderingInstances
object parallel extends ParallelInstances
Expand All @@ -33,12 +32,11 @@ package object instances {
object short extends ShortInstances
object sortedMap extends SortedMapInstances
object sortedSet extends SortedSetInstances
object stream extends StreamInstances
object stream extends StreamInstances with StreamInstancesBinCompat0
object string extends StringInstances
object try_ extends TryInstances
object tuple extends TupleInstances
with Tuple2InstancesBinCompat0
object tuple extends TupleInstances with Tuple2InstancesBinCompat0
object unit extends UnitInstances
object uuid extends UUIDInstances
object vector extends VectorInstances
object vector extends VectorInstances with VectorInstancesBinCompat0
}
Loading