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 traverseCollect to TraverseFilter typeclass #4277

Merged
merged 16 commits into from
Aug 17, 2022
15 changes: 15 additions & 0 deletions core/src/main/scala/cats/TraverseFilter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
package cats

import cats.data.State
import cats.implicits.toTraverseOps
Copy link
Member

Choose a reason for hiding this comment

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

Can we write this without the ops import? Current practice is try to avoid circular dependencies between syntax and typeclasses.

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 tried to do manual sequence for this operation. WDYT?


import scala.collection.immutable.{HashSet, TreeSet}

Expand Down Expand Up @@ -56,6 +57,20 @@ trait TraverseFilter[F[_]] extends FunctorFilter[F] {
*/
def traverseFilter[G[_], A, B](fa: F[A])(f: A => G[Option[B]])(implicit G: Applicative[G]): G[F[B]]

/**
* A combined [[traverse]] and [[collect]].
emilhotkowski marked this conversation as resolved.
Show resolved Hide resolved
*
* scala> import cats.implicits._
* scala> val m: Map[Int, String] = Map(1 -> "one", 2 -> "two")
* scala> val l: List[Int] = List(1, 2, 3, 4)
* scala> def asString: PartialFunction[Int, Eval[Option[String]]] = { case n if n % 2 == 0 => Now(m.get(n)) }
* scala> val result: Eval[List[String]] = l.traverseCollect(asString)
* scala> result.value
* res0: List[String] = List(two)
*/
def traverseCollect[G[_], A, B](fa: F[A])(f: PartialFunction[A, G[B]])(implicit G: Applicative[G]): G[F[B]] =
traverseFilter(fa)(a => f.lift(a).sequence)

/**
* {{{
* scala> import cats.implicits._
Expand Down
12 changes: 12 additions & 0 deletions core/src/main/scala/cats/syntax/traverseFilter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ trait TraverseFilterSyntax extends TraverseFilter.ToTraverseFilterOps
private[syntax] trait TraverseFilterSyntaxBinCompat0 {
implicit def toSequenceFilterOps[F[_], G[_], A](fgoa: F[G[Option[A]]]): SequenceFilterOps[F, G, A] =
new SequenceFilterOps(fgoa)

implicit def toTraverseCollectOps[F[_], G[_], A](fa: F[A]): TraverseCollectOps[F, G, A] =
new TraverseCollectOps(fa)
Copy link
Member

Choose a reason for hiding this comment

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

Should this just be TraverseFilterOps? We can add more ops in the future; it's not specific to traverseCollect I think.

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 followed current implementations like new SequenceFilterOps(fgoa) I don't know what would be the other solution tbh

Copy link
Member

Choose a reason for hiding this comment

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

Ah the situation for SequenceFilter is slighly different, since it only works for F[G[Option[A]]], not F[A]

implicit def toSequenceFilterOps[F[_], G[_], A](fgoa: F[G[Option[A]]]): SequenceFilterOps[F, G, A] =

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you know where I could find examples to follow? I am not sure what I need to change here.

Copy link
Member

@armanbilge armanbilge Aug 16, 2022

Choose a reason for hiding this comment

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

Sure, check out any of the syntax implementations :)
https://github.com/typelevel/cats/tree/main/core/src/main/scala/cats/syntax

I'm just proposing that we rename TraverseCollectOps to TraverseFilterOps so that it matches the name of the typeclass and not the operation.

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 understand your point now. I thought it was a remark about implementation details. Thanks for your work on this PR !

Copy link
Member

Choose a reason for hiding this comment

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

Ah, sorry I wasn't so clear on that. Great, thank you too!

}

final class SequenceFilterOps[F[_], G[_], A](private val fgoa: F[G[Option[A]]]) extends AnyVal {
Expand All @@ -41,3 +44,12 @@ final class SequenceFilterOps[F[_], G[_], A](private val fgoa: F[G[Option[A]]])
*/
def sequenceFilter(implicit F: TraverseFilter[F], G: Applicative[G]): G[F[A]] = F.sequenceFilter(fgoa)
}

final class TraverseCollectOps[F[_], G[_], A](private val fa: F[A]) extends AnyVal {

def traverseCollect[B](f: PartialFunction[A, G[B]])(implicit
F: TraverseFilter[F],
G: Applicative[G]
): G[F[B]] =
F.traverseCollect(fa)(f)
}
8 changes: 8 additions & 0 deletions laws/src/main/scala/cats/laws/TraverseFilterLaws.scala
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ trait TraverseFilterLaws[F[_]] extends FunctorFilterLaws[F] {
G: Monad[G]
): IsEq[G[F[B]]] =
fa.traverseEither(a => f(a).map(_.toRight(e)))((_, _) => Applicative[G].unit) <-> fa.traverseFilter(f)

def traverseCollectRef[G[_], A, B](
emilhotkowski marked this conversation as resolved.
Show resolved Hide resolved
fa: F[A]
)(f: PartialFunction[A, G[B]])(implicit G: Applicative[G]): IsEq[G[F[B]]] = {
val lhs = fa.traverseCollect(f)
val rhs = fa.traverseFilter(a => f.lift(a).sequence)
lhs <-> rhs
}
}

object TraverseFilterLaws {
Expand Down
7 changes: 7 additions & 0 deletions tests/shared/src/test/scala/cats/tests/SyntaxSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -580,4 +580,11 @@ object SyntaxSuite {

val result: Either[A, List[B]] = f.sequenceFilter
}

def testTraverseCollect[A, B]: Unit = {
val list = mock[List[A]]
val f = mock[PartialFunction[A, Option[B]]]

val result: Option[List[B]] = list.traverseCollect(f)
}
}