Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Syntax extensions for Parallel require Monad[M], but implementation doesn't #2254

Closed
oleg-py opened this issue May 10, 2018 · 3 comments · Fixed by #3856
Closed

Syntax extensions for Parallel require Monad[M], but implementation doesn't #2254

oleg-py opened this issue May 10, 2018 · 3 comments · Fixed by #3856

Comments

@oleg-py
Copy link

oleg-py commented May 10, 2018

I discovered today that to use syntax extension for parTraverse/parSequence, a Monad[M] instance is required.

final class ParallelTraversableOps[T[_], A](val ta: T[A]) extends AnyVal {

  def parTraverse[M[_]: Monad, F[_], B]
  (f: A => M[B])(implicit T: Traverse[T], P: Parallel[M, F]): M[T[B]] =
    Parallel.parTraverse(ta)(f)

}

While Parallel.parTraverse is defined as follows (notice lack of Monad[M])

  def parTraverse[T[_]: Traverse, M[_], F[_], A, B]
  (ta: T[A])(f: A => M[B])(implicit P: Parallel[M, F]): M[T[B]] = {
    val gtb: F[T[B]] = Traverse[T].traverse(ta)(f andThen P.parallel.apply)(P.applicative)
    P.sequential(gtb)
  }

Same is true for parSequence. Is that an oversight?

@LukaJCB
Copy link
Member

LukaJCB commented May 10, 2018

Yes it is, unfortunately we can't change it until cats 2.0.
There's already a PR at #2180

@tnielens
Copy link
Contributor

tnielens commented Apr 8, 2021

Seems like this issue missed cats 2.0 opportunity. Might be worth labeling it with 3.0.

@joroKr21
Copy link
Member

joroKr21 commented Apr 8, 2021

Why can't we change it? Implicits can be changed in a binary compatible way by making the old ones non-implicit and adding the new ones.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants