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

Adding FunctionK values for Task, Coeval #865

Merged
merged 32 commits into from Jun 16, 2019

Conversation

Projects
None yet
3 participants
@alexandru
Copy link
Member

commented Apr 18, 2019

I'm using Coeval lately, for describing resources like Resource[Coeval, ?] and then I'm converting those to Task when needed. This is because you may want to initialize a resource in a dirty context, like Play Framework's dirty dependency injection and lifecycle mechanism and working with Coeval there is easier and more truthful.

But then doing a mapK to convert from Resource[Coeval, ?] to Resource[Task, ?] is freaking painful. Therefore we now have liftTo and liftFrom to help:

val res: Resource[Coeval, Something] = ???

res.mapK(Coeval.liftTo[Task]).use { r =>
  Task(???)
}

//------

val res: Resource[SyncIO, Something] = ???

res.mapK(Coeval.liftFrom[SyncIO]).use { r =>
  Task(???)
}

Summary of changes:

  1. added CoevalLift
  2. changed TaskLift, TaskLike, ObservableLike and FutureLift to extend FunctionK[F, G] (colloquially F ~> G)
  3. added liftTo, liftFrom to Coeval and Task, also added liftFrom to Observable for consistency
  4. deprecated toIO / fromIO and fromEval across the board
  5. renamed liftMap from Iterant to mapK, for consistency with Resource in Cats-Effect

1. CoevalLift

This mirrors TaskLift and LiftIO from cats.effect and is used for lifting Coeval values into something else:

trait CoevalLift[F[_]] extends (Coeval ~> F)

Thus we can now do:

Coeval(1 + 1).to[SyncIO]

Coeval(1 + 1).to[Task]

2. TaskLift, TaskLike, ObservableLike

I changed them across the board to extend FunctionK from Cats:

trait CoevalLift[F[_]] extends (Coeval ~> F)

trait CoevalLike[F[_]] extends (F ~> Coeval)

trait TaskLift[F[_]] extends (Task ~> F)

trait TaskLike[F[_]] extends (F ~> Task)

trait ObservableLike[F[_]] extends (F ~> Observable)

trait FutureLift[F[_], Future[_]] extends (Lambda[A => F[Future[A]]] ~> F)

3. Added liftTo / liftFrom and variants

For Coeval:

object Coeval {

  def liftTo[F[_]](implicit F: CoevalLift[F]): Coeval ~> F = F

  def liftToSync[F[_]](implicit F: Sync[F]): Coeval ~> F

  def liftFrom[F[_]](implicit F: CoevalLike[F]): F ~> Coeval

}

For Task:

object Task {

  def liftTo[F[_]](implicit F: TaskLift[F]): (Task ~> F) = F

  def liftToAsync[F[_]](implicit F: cats.effect.Async[F], eff: cats.effect.Effect[Task]): (Task ~> F)

  def liftToConcurrent[F[_]](
    implicit F: cats.effect.Concurrent[F],
    eff: cats.effect.ConcurrentEffect[Task]): (Task ~> F)

  def liftFrom[F[_]](implicit F: TaskLike[F]): (F ~> Task)

  def liftFromEffect[F[_]](implicit F: Effect[F]): (F ~> Task)

  def liftFromConcurrentEffect[F[_]](implicit F: ConcurrentEffect[F]): (F ~> Task)

}

For Observable:

object Observable {

  def liftFrom[F[_]](implicit F: ObservableLike[F]): F ~> Observable

}

3. Deprecated .toIO, fromIO, fromEval across the board

These functions are not special, IO will soon no longer live in the base Cats-Effect package and Eval isn't so interesting as to have its own method.

Conversions for all of them are still supported via Coeval.from / Task.from / Observable.from and coeval.to / task.to.

5. Renamed liftMap from Iterant to mapK

Something that's bugging me a lot is naming consistency. Cats-Effect's Resource has mapK, but Iterant has liftMap. I don't like mapK that much, but consistency is important and I'd rather have naming consistency with Resource.

Old name is now a deprecated symbol.

alexandru added some commits Jul 17, 2017

*/
@implicitNotFound("""Cannot find implicit value for CoevalLift[${F}].
Building this implicit value might depend on having an implicit
s.c.ExecutionContext in scope, a Scheduler or some equivalent type.""")

This comment has been minimized.

Copy link
@oleg-py

oleg-py Apr 18, 2019

Collaborator

Is there any type in the ecosystem that needs EC for a Sync instance? I'm afraid that might be more misleading than helpful.

This comment has been minimized.

Copy link
@alexandru

alexandru Apr 18, 2019

Author Member

No, just a left-over

@implicitNotFound("""Cannot find implicit value for CoevalLift[${F}].
Building this implicit value might depend on having an implicit
s.c.ExecutionContext in scope, a Scheduler or some equivalent type.""")
trait CoevalLift[F[_]] {

This comment has been minimized.

Copy link
@oleg-py

oleg-py Apr 18, 2019

Collaborator

Proposal:

trait CoevalLift[F[_]] extends Coeval ~> F // no body

Might cut out on the boilerplate in the impls and will show the intent in the code too

This comment has been minimized.

Copy link
@alexandru

alexandru Apr 18, 2019

Author Member

Sounds interesting 👍

* See [[https://typelevel.org/cats/datatypes/functionk.html]].
*/
def toK[F[_]](implicit F: TaskLift[F]): FunctionK[Task, F] =
new FunctionK[Task, F] {

This comment has been minimized.

Copy link
@oleg-py

oleg-py Apr 18, 2019

Collaborator

There's a macro method on FunctionK for this, IIRC FunctionK.lift(F.taskLift)
Also, kind-projector should let you write λ[Task ~> F](F.taskLift) too

@oleg-py

This comment has been minimized.

Copy link
Collaborator

commented Apr 18, 2019

TBH, I'm not very big fan of lawless typeclasses, and wins overres.mapK(FunctionK.lift(Task.coeval)) seem marginal, though I'd take to[F[_]: Sync] over toIO.

@alexandru

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2019

@oleg-py I'm not a fan of lawless type classes either, but due to all the restrictions in Cats-Effect's Bracket and Sync, there are now many interesting types that are left out.

Eval and Observable are two such examples.

@alexandru

This comment has been minimized.

Copy link
Member Author

commented Apr 22, 2019

@oleg-py reliance on FunctorK.lift doesn't cut it, as you can easily end up with an error like this ...

missing parameter type for expanded function ((x$1: <error>) => x$1.task)
[error]       .mapK(FunctionK.lift(_.task))

The type inference does work with FunctionK.lift(Task.coeval), but I don't think many people will know that. And there is no such function already described for cats.effect.IO.

Will take your suggestions under consideration. I especially like the idea of making the *Lift type classes inherit from FunctorK.

alexandru added some commits Jun 12, 2019

@@ -59,8 +60,8 @@ import scala.concurrent.{Future => ScalaFuture}
* val sum: IO[Int] = IO(delayed(1 + 1)).futureLift
* }}}
*/
trait FutureLift[F[_], Future[_]] {

This comment has been minimized.

Copy link
@Avasil

Avasil Jun 13, 2019

Collaborator

A bit off-topic but it reminded me:

typelevel/cats-effect#546

Cats-Effect changed fromFuture to do shift. Should we reconsider it on our part too?

This comment has been minimized.

Copy link
@alexandru

alexandru Jun 13, 2019

Author Member

Yes, maybe.

@alexandru alexandru changed the title Adding FunctionK values for Task, Coeval WIP: Adding FunctionK values for Task, Coeval Jun 13, 2019

alexandru added some commits Jun 13, 2019

@alexandru alexandru changed the title WIP: Adding FunctionK values for Task, Coeval Adding FunctionK values for Task, Coeval Jun 13, 2019

alexandru added some commits Jun 13, 2019

@alexandru alexandru requested review from oleg-py and Avasil Jun 13, 2019

@oleg-py
Copy link
Collaborator

left a comment

Looks good. I can only nitpick 😄

@@ -119,7 +134,7 @@ private[eval] abstract class CoevalLikeImplicits0 {
*/
implicit def fromComonad[F[_]](implicit F: Comonad[F]): CoevalLike[F] =

This comment has been minimized.

Copy link
@oleg-py

oleg-py Jun 14, 2019

Collaborator

Actually, do we really want this instance? Apart from Eval, standard comonads from Cats are things like NonEmptyList, and a traditional case is something like a zipper, where extract would lose data - not that it makes sense to convert it to Coeval anyway.

This comment has been minimized.

Copy link
@alexandru

alexandru Jun 14, 2019

Author Member

OK, it probably doesn't make sense. Will just treat Eval as a special case and remove this.

implicit class Deprecated[F[_]](val inst: TaskLift[F]) {
/** DEPRECATED — switch to [[TaskLift.apply]]. */
@deprecated("Switch to TaskLift.apply", since = "3.0.0-RC3")
def taskLift[A](task: Task[A]): F[A] = {

This comment has been minimized.

Copy link
@oleg-py

oleg-py Jun 14, 2019

Collaborator

Why not have this as a final method on TaskLift that just delegates to apply? Same for all the Deprecated classes

This comment has been minimized.

Copy link
@alexandru

alexandru Jun 14, 2019

Author Member

Hmm, maybe, thinking about it.

@Avasil

Avasil approved these changes Jun 14, 2019

@Avasil

This comment has been minimized.

Copy link
Collaborator

commented Jun 16, 2019

@alexandru is it ready to merge? I could do the realese today's evening :)

@alexandru

This comment has been minimized.

Copy link
Member Author

commented Jun 16, 2019

@Avasil yes, it's ready

@Avasil Avasil merged commit 3c7dca8 into monix:master Jun 16, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.