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

WIP: Selective #3709

Draft
wants to merge 47 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
444e3bd
Introduce Selective
rossabaker Dec 9, 2020
da7f756
Split into Select and Selective
rossabaker Dec 9, 2020
d220349
Selective[Validated[E, *]]
rossabaker Dec 9, 2020
4c7f87e
SelectiveError
rossabaker Dec 9, 2020
14d3e42
FlatMap extends Select
rossabaker Dec 9, 2020
f554862
Merge branch 'master' into selective
rossabaker Dec 9, 2020
b7112ac
Required parens on lambda
rossabaker Dec 9, 2020
b8dc63b
Default select from Apply until Monad
rossabaker Dec 9, 2020
7fd409b
Start writing SelectiveLaws
rossabaker Dec 9, 2020
254bf7f
Look at me, not compiling before I push
rossabaker Dec 9, 2020
ed71f06
Never mind on Select
rossabaker Dec 9, 2020
7767906
branch, ifS, whenS
rossabaker Dec 9, 2020
466354b
select is an abstract member of Selective
rossabaker Dec 10, 2020
f31020a
Law for monad select rigidity
rossabaker Dec 10, 2020
4a2b0be
scalafmt
rossabaker Dec 10, 2020
955a03b
Use our new syntax
rossabaker Dec 10, 2020
e35a097
Test default operations
rossabaker Dec 10, 2020
df699d1
Derive missing implicits for selective laws
rossabaker Dec 14, 2020
a50da48
Remove SelectiveError for now
rossabaker Dec 14, 2020
387ab75
Selective distributivity and associativity
rossabaker Dec 14, 2020
cb51502
Clean up syntax
rossabaker Dec 14, 2020
06abe62
apS and RigidSelective laws
rossabaker Dec 16, 2020
9b001dd
Remove inner scope in branch
rossabaker Dec 16, 2020
19b67b4
Move EitherUtil to root package for use in typeclasses
rossabaker Dec 16, 2020
662cf5a
Use cached either units in ifS
rossabaker Dec 16, 2020
936847f
Fix loop in laws inheritance
rossabaker Dec 16, 2020
3372948
Remove unnecessary private[cats] in EitherUtil
rossabaker Dec 16, 2020
a866a20
Apply.selectA
rossabaker Dec 16, 2020
260a219
select's function is lazy; skip on right law for rigids
rossabaker Dec 16, 2020
a0f8709
Skip effects in branch and ifS in rigid selectives
rossabaker Dec 16, 2020
37594b9
Skip effect of whenS when false in rigid selectives
rossabaker Dec 16, 2020
b56130d
Replace apS with RigidSelective typeclass
rossabaker Dec 16, 2020
9079e16
Validated is rigid
rossabaker Dec 17, 2020
020a9fb
Revert "Validated is rigid" -- it's not rigid yet
rossabaker Dec 19, 2020
92f262f
Push select down to Apply
rossabaker Dec 19, 2020
762dd7d
Push branch and ifS down to Apply
rossabaker Dec 19, 2020
691144e
Push whenS down to Applicative
rossabaker Dec 19, 2020
3e31609
Push identity and distributivity laws to Applicative
rossabaker Dec 19, 2020
6b496ec
Remove Selective
rossabaker Dec 19, 2020
acce38c
Remove commented implicits
rossabaker Dec 20, 2020
7c78d54
RigidSelective default ap causes loops
rossabaker Dec 20, 2020
c4c1442
Add back lost selective associativity test
rossabaker Dec 20, 2020
3bcb791
Restate ifS skip laws to accommodate Under
rossabaker Dec 20, 2020
dba3dc0
Selective has re-entered the chat
rossabaker Dec 21, 2020
7a4d66a
Selective[ZipLazyList]
rossabaker Dec 21, 2020
6616707
Selective[Func] and RigidSelective[Func]
rossabaker Dec 21, 2020
bd0d4d9
Fix name of selective laws
rossabaker Dec 21, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion core/src/main/scala/cats/Applicative.scala
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ import scala.annotation.implicitNotFound
*/
def whenA[A](cond: Boolean)(f: => F[A]): F[Unit] =
if (cond) void(f) else unit

}

object Applicative {
Expand Down
30 changes: 18 additions & 12 deletions core/src/main/scala/cats/Apply.scala
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,12 @@ trait Apply[F[_]] extends Functor[F] with InvariantSemigroupal[F] with ApplyArit
def ite(b: Boolean)(ifTrue: A, ifFalse: A) = if (b) ifTrue else ifFalse
ap2(map(fcond)(ite))(ifTrue, ifFalse)
}

def selectA[A, B](fab: F[Either[A, B]])(ff: F[A => B]): F[B] =
Copy link
Contributor

Choose a reason for hiding this comment

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

Remind me again why we need both select and selectA if setting them equal is lawful? Why not just add select which may or may not be override to be rigid?

Copy link
Member Author

Choose a reason for hiding this comment

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

  • selectA is like ifA: requires 'Apply, not rigid even for FlatMap`s
  • selectM (not added yet) is like ifM: requires FlatMap, rigid
  • select is like ifS: requires 'Apply, rigid for FlatMap`s, otherwise effect's choice

I can live without selectA and selectM, but their existence parallels some other functions. (I'll note that @SystemFw just dumped on ifA on Gitter today.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect ifA without rigidity is a recipe for exponential blow ups in cost.

Copy link
Contributor

Choose a reason for hiding this comment

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

(I'll note that @SystemFw just dumped on ifA on Gitter today.

I have barely skimmed the whole conversation, so I do apologise for the lack of context, but I do think ifA is one of the worse things added in cats.
An if that executes both branches is what you want 0.01% of the time, and users have been told to "use the least powerful constraint you can", which makes it a recipe for disaster. We can't even claim we didn't see this, because the if + macro interaction in sbt has been hated for the same reason for years.

Personally I see Selective as just pulling things down from Monad, to get you more static analysis, so in an ideal world ifA would be gone (and similarly for select* on Apply), Selective would called just that, and have ifM (ifS?), etc. So (barring misunderstanding of what I've skimmed), I think I'm largely in agreement with @johnynek

Copy link
Contributor

Choose a reason for hiding this comment

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

So, I caused a bit of the wild goose chase here I think.

One thing I was and am concerned about is a lawful implementation of Selective using Applicative, why not add to Applicative? So, I pushed to lower things to Applicative or Apply. But that means the default implementation winds up executing both sides, but as you note, you would almost never want that. It seems weird to open the door for exactly one use case that we can name (over approximating dependencies in a modestly dynamic graph).

So, now my thinking has gotten to either:

  1. Reintroduce Selective (basically back where we started, yes you could implement Selective with Applicative, but it is probably not what you want, so it isn't a default, this allows you to implement it when you are sure that is what you want).
  2. I'm dubious RigidSelective is much of a thing. I tend to wonder if the only instances are things that could be monads but are being restricted in some way (e.g. CLI parsers being restricted so that they can generate a static help).
  3. A conservative way out would be to add def select to Monad and implement it with flatMap, add the select laws there (and require it to be rigid). This allows Monad instances to optimize select if they can (like Parser or Gen could).

So, one idea would be:

// Comment that this is a rarely used type that is generally used to make static analysis of
// F[_] values that model a deferred computation.
trait Selective[F[_]] extends Applicative[F] {
  def select[A, B](e: F[Either[A, B]])(fn: F[A => B]): F[B]
}

trait RigidSelective[F[_]] extends Selective[F] {
  def ifM... // all the monad methods that can be implemented in terms of select
}

trait Monad[F[_]] extends RigidSelective[F] {
...
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Great summary Ross.

To simplify, I think ZipList, NonEmptyList.ZipNonEmptyList, Validated, IO.Par and Nested are the core things to think about. Clearly if Zip*List works, the other collections which are isomorphic should work too.

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 need to cook now, but quick response to "So, one idea would be:..."

  • Yes, let's set aside the difference between Apply and Applicative for now. We can circle back on maps and tuples in a second PR, or not, after we're done learning from this.
  • Agree with reviving Selective[F]. Several of the laws (identity, associativity, distributivity) can stick there.
  • Agree on monad having rigidity laws. (That has remained constant through this PR.)

I have some qualms pulling the *M functions down to RigidSelective:

  1. As you note, few things are without being monads. Which things? Depends whether we stick to our skip laws, or accept the ap == apS law, or both.
  2. Depending how we resolve the above, some things may have a viable ifM without being a RigidSelective, but we'd have no typeclass for these. We could pull those functions down to Selective, but the name would lose its current sense of rigidity.
  3. Some of the'*M functions are on FlatMap. Now we need to think about that relationship earlier than we'd like.

Haskell gets around all these problems with an *S set of functions that's usually like *M. Great for laws, confusing for new users autocompleting weird names.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oof. ZipList is in the pile of things that are Apply but not Applicative that I'm trying to forget about for now.

ZipStream, ZipLazyList, and IO.Par are all Applicative.

Copy link
Member Author

@rossabaker rossabaker Dec 21, 2020

Choose a reason for hiding this comment

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

The Applicative[ZipLazyList] laws don't terminate, so I don't know if our Selective[ZipLazyList] is lawful. But it appears to be Haskell-rigid: the Apply laws pass when ap is implemented as apS. It's not Cats-Issue-3709-rigid, because we have to evaluate ff even if we get a lazy stream of Rights.

Copy link

@j-mie6 j-mie6 Jan 2, 2023

Choose a reason for hiding this comment

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

  1. I'm dubious RigidSelective is much of a thing. I tend to wonder if the only instances are things that could be monads but are being restricted in some way (e.g. CLI parsers being restricted so that they can generate a static help).

One very good reason for having a specific Selective thing that isn't also a monad is precisely to leverage static structure with dynamic behaviours. Take parsers, for example: Applicative isn't strong enough to do context-sensitive things, Monad is, and Selective gives you some (of the most common) context-sensitive behaviours but not all of them. This is important, because if you wanted to generate fast hand-written recursive descent for a parser combinator implementation, you need a fully static structure. That rules out Monad, but not Selective or Applicative: luckily these can almost always be enough for your practical parsing needs, but such a library wouldn't be able to make a Monad instance (see https://github.com/j-mie6/ParsleyHaskell as an example of such a library). Basically, one killer application is for staged DSLs, which can't be monadic without runtime code-generation.

map2(fab, ff) {
case (Left(a), f) => f(a)
case (Right(b), _) => b
}
}

object Apply {
Expand Down Expand Up @@ -289,12 +295,11 @@ object Apply {
object ops {
implicit def toAllApplyOps[F[_], A](target: F[A])(implicit tc: Apply[F]): AllOps[F, A] {
type TypeClassType = Apply[F]
} =
new AllOps[F, A] {
type TypeClassType = Apply[F]
val self: F[A] = target
val typeClassInstance: TypeClassType = tc
}
} = new AllOps[F, A] {
type TypeClassType = Apply[F]
val self: F[A] = target
val typeClassInstance: TypeClassType = tc
}
}
trait Ops[F[_], A] extends Serializable {
type TypeClassType <: Apply[F]
Expand All @@ -312,19 +317,20 @@ object Apply {
typeClassInstance.ap2[B, C, D](self.asInstanceOf[F[(B, C) => D]])(fa, fb)
def map2[B, C](fb: F[B])(f: (A, B) => C): F[C] = typeClassInstance.map2[A, B, C](self, fb)(f)
def map2Eval[B, C](fb: Eval[F[B]])(f: (A, B) => C): Eval[F[C]] = typeClassInstance.map2Eval[A, B, C](self, fb)(f)
def selectA[B, C](ff: F[B => C])(implicit ev$1: A <:< Either[B, C]): F[C] =
typeClassInstance.selectA[B, C](self.asInstanceOf[F[Either[B, C]]])(ff)
}
trait AllOps[F[_], A] extends Ops[F, A] with Functor.AllOps[F, A] with InvariantSemigroupal.AllOps[F, A] {
type TypeClassType <: Apply[F]
}
trait ToApplyOps extends Serializable {
implicit def toApplyOps[F[_], A](target: F[A])(implicit tc: Apply[F]): Ops[F, A] {
type TypeClassType = Apply[F]
} =
new Ops[F, A] {
type TypeClassType = Apply[F]
val self: F[A] = target
val typeClassInstance: TypeClassType = tc
}
} = new Ops[F, A] {
type TypeClassType = Apply[F]
val self: F[A] = target
val typeClassInstance: TypeClassType = tc
}
}
@deprecated("Use cats.syntax object imports", "2.2.0")
object nonInheritedOps extends ToApplyOps
Expand Down
14 changes: 14 additions & 0 deletions core/src/main/scala/cats/EitherUtil.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package cats

/**
* Convenience methods and values for Either.
*/
private[cats] object EitherUtil {
def leftCast[A, B, C](right: Right[A, B]): Either[C, B] =
right.asInstanceOf[Either[C, B]]
def rightCast[A, B, C](left: Left[A, B]): Either[A, C] =
left.asInstanceOf[Either[A, C]]

val unit = Right(())
val leftUnit = Left(())
}
32 changes: 18 additions & 14 deletions core/src/main/scala/cats/Monad.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,16 @@ import scala.annotation.implicitNotFound
* Must obey the laws defined in cats.laws.MonadLaws.
*/
@implicitNotFound("Could not find an instance of Monad for ${F}")
@typeclass trait Monad[F[_]] extends FlatMap[F] with Applicative[F] {
@typeclass trait Monad[F[_]] extends FlatMap[F] with RigidSelective[F] {
override def map[A, B](fa: F[A])(f: A => B): F[B] =
flatMap(fa)(a => pure(f(a)))

override def select[A, B](fab: F[Either[A, B]])(ff: => F[A => B]): F[B] =
flatMap(fab) {
case Left(a) => map(ff)(_(a))
case Right(b) => pure(b)
}

/**
* Execute an action repeatedly as long as the given `Boolean` expression
* returns `true`. The condition is evaluated before the loop body.
Expand Down Expand Up @@ -161,12 +167,11 @@ object Monad {
object ops {
implicit def toAllMonadOps[F[_], A](target: F[A])(implicit tc: Monad[F]): AllOps[F, A] {
type TypeClassType = Monad[F]
} =
new AllOps[F, A] {
type TypeClassType = Monad[F]
val self: F[A] = target
val typeClassInstance: TypeClassType = tc
}
} = new AllOps[F, A] {
type TypeClassType = Monad[F]
val self: F[A] = target
val typeClassInstance: TypeClassType = tc
}
}
trait Ops[F[_], A] extends Serializable {
type TypeClassType <: Monad[F]
Expand All @@ -178,18 +183,17 @@ object Monad {
def iterateWhile(p: A => Boolean): F[A] = typeClassInstance.iterateWhile[A](self)(p)
def iterateUntil(p: A => Boolean): F[A] = typeClassInstance.iterateUntil[A](self)(p)
}
trait AllOps[F[_], A] extends Ops[F, A] with FlatMap.AllOps[F, A] with Applicative.AllOps[F, A] {
trait AllOps[F[_], A] extends Ops[F, A] with FlatMap.AllOps[F, A] with Selective.AllOps[F, A] {
type TypeClassType <: Monad[F]
}
trait ToMonadOps extends Serializable {
implicit def toMonadOps[F[_], A](target: F[A])(implicit tc: Monad[F]): Ops[F, A] {
type TypeClassType = Monad[F]
} =
new Ops[F, A] {
type TypeClassType = Monad[F]
val self: F[A] = target
val typeClassInstance: TypeClassType = tc
}
} = new Ops[F, A] {
type TypeClassType = Monad[F]
val self: F[A] = target
val typeClassInstance: TypeClassType = tc
}
}
@deprecated("Use cats.syntax object imports", "2.2.0")
object nonInheritedOps extends ToMonadOps
Expand Down
59 changes: 59 additions & 0 deletions core/src/main/scala/cats/RigidSelective.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package cats

import simulacrum.typeclass
import scala.annotation.implicitNotFound

@implicitNotFound("Could not find an instance of RigidSelective for ${F}")
@typeclass trait RigidSelective[F[_]] extends Selective[F] {
override def ap[A, B](ff: F[A => B])(fa: F[A]): F[B] = {
val left: F[Either[A => B, B]] = map(ff)(Left(_))
val right: F[(A => B) => B] = map(fa)((a: A) => _(a))
select(left)(right)
}
}

object RigidSelective {
/* ======================================================================== */
/* THE FOLLOWING CODE IS MANAGED BY SIMULACRUM; PLEASE DO NOT EDIT!!!! */
/* ======================================================================== */

/**
* Summon an instance of [[RigidSelective]] for `F`.
*/
@inline def apply[F[_]](implicit instance: RigidSelective[F]): RigidSelective[F] = instance

@deprecated("Use cats.syntax object imports", "2.2.0")
object ops {
implicit def toAllRigidSelectiveOps[F[_], A](target: F[A])(implicit tc: RigidSelective[F]): AllOps[F, A] {
type TypeClassType = RigidSelective[F]
} = new AllOps[F, A] {
type TypeClassType = RigidSelective[F]
val self: F[A] = target
val typeClassInstance: TypeClassType = tc
}
}
trait Ops[F[_], A] extends Serializable {
type TypeClassType <: RigidSelective[F]
def self: F[A]
val typeClassInstance: TypeClassType
}
trait AllOps[F[_], A] extends Ops[F, A] with Selective.AllOps[F, A] {
type TypeClassType <: RigidSelective[F]
}
trait ToRigidSelectiveOps extends Serializable {
implicit def toRigidSelectiveOps[F[_], A](target: F[A])(implicit tc: RigidSelective[F]): Ops[F, A] {
type TypeClassType = RigidSelective[F]
} = new Ops[F, A] {
type TypeClassType = RigidSelective[F]
val self: F[A] = target
val typeClassInstance: TypeClassType = tc
}
}
@deprecated("Use cats.syntax object imports", "2.2.0")
object nonInheritedOps extends ToRigidSelectiveOps

/* ======================================================================== */
/* END OF SIMULACRUM-MANAGED CODE */
/* ======================================================================== */

}
78 changes: 78 additions & 0 deletions core/src/main/scala/cats/Selective.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package cats

import simulacrum.{noop, typeclass}
import scala.annotation.implicitNotFound

@implicitNotFound("Could not find an instance of Selective for ${F}")
@typeclass trait Selective[F[_]] extends Applicative[F] {
rossabaker marked this conversation as resolved.
Show resolved Hide resolved
def select[A, B](fab: F[Either[A, B]])(ff: => F[A => B]): F[B]

def branch[A, B, C](fab: F[Either[A, B]])(fl: => F[A => C])(fr: => F[B => C]): F[C] = {
val innerLhs: F[Either[A, Either[B, C]]] = map(fab)(_.map(Left(_)))
def innerRhs: F[A => Either[B, C]] = map(fl)(_.andThen(Right(_)))
val lhs = select(innerLhs)(innerRhs)
select(lhs)(fr)
}

@noop
def ifS[A](fCond: F[Boolean])(fTrue: => F[A])(fFalse: => F[A]): F[A] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is the exact same signature as ifM. Why can't we use the same name?

This is going to be really confusing. I would rather just name this method ifM.

Copy link
Member Author

Choose a reason for hiding this comment

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

If this were a clean slate, we'd pull ifM down and call it ifS. ifM is equivalent to ifS where it presently exists, but if we pull it down, some implementations will behave like ifA, and we'd be unexpectedly changing the behavior of an established name.

I would like to call it ifS and deprecate ifM, but I'm not sure how bad the warnings would be.

val condition: F[Either[Unit, Unit]] = map(fCond)(if (_) EitherUtil.leftUnit else EitherUtil.unit)
def left: F[Unit => A] = map(fTrue)(Function.const)
def right: F[Unit => A] = map(fFalse)(Function.const)
branch(condition)(left)(right)
}

@noop
def whenS[A](fCond: F[Boolean])(fTrue: => F[Unit]): F[Unit] =
ifS(fCond)(fTrue)(unit)
}

object Selective {
/* ======================================================================== */
/* THE FOLLOWING CODE IS MANAGED BY SIMULACRUM; PLEASE DO NOT EDIT!!!! */
/* ======================================================================== */

/**
* Summon an instance of [[Selective]] for `F`.
*/
@inline def apply[F[_]](implicit instance: Selective[F]): Selective[F] = instance

@deprecated("Use cats.syntax object imports", "2.2.0")
object ops {
implicit def toAllSelectiveOps[F[_], A](target: F[A])(implicit tc: Selective[F]): AllOps[F, A] {
type TypeClassType = Selective[F]
} = new AllOps[F, A] {
type TypeClassType = Selective[F]
val self: F[A] = target
val typeClassInstance: TypeClassType = tc
}
}
trait Ops[F[_], A] extends Serializable {
type TypeClassType <: Selective[F]
def self: F[A]
val typeClassInstance: TypeClassType
def select[B, C](ff: => F[B => C])(implicit ev$1: A <:< Either[B, C]): F[C] =
typeClassInstance.select[B, C](self.asInstanceOf[F[Either[B, C]]])(ff)
def branch[B, C, D](fl: => F[B => D])(fr: => F[C => D])(implicit ev$1: A <:< Either[B, C]): F[D] =
typeClassInstance.branch[B, C, D](self.asInstanceOf[F[Either[B, C]]])(fl)(fr)
}
trait AllOps[F[_], A] extends Ops[F, A] with Applicative.AllOps[F, A] {
type TypeClassType <: Selective[F]
}
trait ToSelectiveOps extends Serializable {
implicit def toSelectiveOps[F[_], A](target: F[A])(implicit tc: Selective[F]): Ops[F, A] {
type TypeClassType = Selective[F]
} = new Ops[F, A] {
type TypeClassType = Selective[F]
val self: F[A] = target
val typeClassInstance: TypeClassType = tc
}
}
@deprecated("Use cats.syntax object imports", "2.2.0")
object nonInheritedOps extends ToSelectiveOps

/* ======================================================================== */
/* END OF SIMULACRUM-MANAGED CODE */
/* ======================================================================== */

}
6 changes: 3 additions & 3 deletions core/src/main/scala/cats/data/EitherT.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package cats
package data

import cats.Bifunctor
import cats.syntax.EitherUtil
import cats.EitherUtil

/**
* Transformer for `Either`, allowing the effect of an arbitrary type constructor `F` to be combined with the
Expand Down Expand Up @@ -922,7 +922,7 @@ abstract private[data] class EitherTInstances extends EitherTInstances1 {
implicit val monadEither: Monad[Either[E, *]] = cats.instances.either.catsStdInstancesForEither

def applicative: Applicative[Nested[P.F, Validated[E, *], *]] =
cats.data.Nested.catsDataApplicativeForNested(P.applicative, Validated.catsDataApplicativeErrorForValidated)
cats.data.Nested.catsDataApplicativeForNested(P.applicative, Validated.catsDataSelectiveErrorForValidated)

def monad: Monad[EitherT[M, E, *]] = cats.data.EitherT.catsDataMonadErrorForEitherT

Expand Down Expand Up @@ -983,7 +983,7 @@ abstract private[data] class EitherTInstances1 extends EitherTInstances2 {
new Parallel[EitherT[M, E, *]] {
type F[x] = Nested[M, Validated[E, *], x]

implicit val appValidated: Applicative[Validated[E, *]] = Validated.catsDataApplicativeErrorForValidated
implicit val appValidated: Applicative[Validated[E, *]] = Validated.catsDataSelectiveErrorForValidated
implicit val monadEither: Monad[Either[E, *]] = cats.instances.either.catsStdInstancesForEither

def applicative: Applicative[Nested[M, Validated[E, *], *]] =
Expand Down
30 changes: 24 additions & 6 deletions core/src/main/scala/cats/data/Validated.scala
Original file line number Diff line number Diff line change
Expand Up @@ -931,16 +931,21 @@ sealed abstract private[data] class ValidatedInstances extends ValidatedInstance
fab.leftMap(f)
}

implicit def catsDataApplicativeErrorForValidated[E](implicit E: Semigroup[E]): ApplicativeError[Validated[E, *], E] =
new ValidatedApplicative[E] with ApplicativeError[Validated[E, *], E] {

implicit def catsDataSelectiveErrorForValidated[E](implicit
E: Semigroup[E]
): Selective[Validated[E, *]] with ApplicativeError[Validated[E, *], E] =
new ValidatedSelective[E] with ApplicativeError[Validated[E, *], E] {
rossabaker marked this conversation as resolved.
Show resolved Hide resolved
def handleErrorWith[A](fa: Validated[E, A])(f: E => Validated[E, A]): Validated[E, A] =
fa match {
case Validated.Invalid(e) => f(e)
case v @ Validated.Valid(_) => v
}
def raiseError[A](e: E): Validated[E, A] = Validated.Invalid(e)
}

@deprecated("Use catsDataSelectiveErrorForValidated", "2.4.0")
def catsDataApplicativeErrorForValidated[E](implicit E: Semigroup[E]): ApplicativeError[Validated[E, *], E] =
catsDataSelectiveErrorForValidated
}

sealed abstract private[data] class ValidatedInstances1 extends ValidatedInstances2 {
Expand All @@ -953,9 +958,13 @@ sealed abstract private[data] class ValidatedInstances1 extends ValidatedInstanc
def combine(x: Validated[A, B], y: Validated[A, B]): Validated[A, B] = x.combine(y)
}

implicit def catsDataCommutativeApplicativeForValidated[E: CommutativeSemigroup]
: CommutativeApplicative[Validated[E, *]] =
new ValidatedApplicative[E] with CommutativeApplicative[Validated[E, *]]
implicit def catsDataCommutativeSelectiveForValidated[E: CommutativeSemigroup]
: Selective[Validated[E, *]] with CommutativeApplicative[Validated[E, *]] =
new ValidatedSelective[E] with CommutativeApplicative[Validated[E, *]]

@deprecated("Use catsDataCommutativeSelectiveForValidated", "2.4.0")
def catsDataCommutativeApplicativeForValidated[E: CommutativeSemigroup]: CommutativeApplicative[Validated[E, *]] =
Copy link
Member Author

Choose a reason for hiding this comment

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

If there's a CommutativeApplicative and a CommutativeMonad, we need to spend a few moments thinking about whether there's a CommutativeSelective.

catsDataCommutativeApplicativeForValidated

implicit def catsDataPartialOrderForValidated[A: PartialOrder, B: PartialOrder]: PartialOrder[Validated[A, B]] =
new PartialOrder[Validated[A, B]] {
Expand Down Expand Up @@ -1035,6 +1044,15 @@ sealed abstract private[data] class ValidatedInstances2 {
// scalastyle:off method.length
}

private[data] class ValidatedSelective[E: Semigroup] extends ValidatedApplicative[E] with Selective[Validated[E, *]] {
override def select[A, B](fab: Validated[E, Either[A, B]])(ff: => Validated[E, A => B]): Validated[E, B] =
Copy link
Contributor

Choose a reason for hiding this comment

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

is this not rigid? Looks like to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

It skips the effect appropriately, but is inconsistent with ap.

This is consistent with ap, but fails the skip laws, which I think could be relaxed to work only on pure fab. More importantly, it fails associativity, and I'm entering the stupid hours.

  override def select[A, B](fab: Validated[E, Either[A, B]])(ff: => Validated[E, A => B]): Validated[E, B] =
    fab match {
      case Valid(Right(b)) => Valid(b)
      case Valid(Left(a)) => ff.map(_(a))
      case e @ Invalid(e1) =>
        ff match {
          case Valid(_) => e
          case Invalid(e2) => Invalid(EE.combine(e1, e2))
        }
    }

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the Invalid - Valid(Right)) - Invalid combination that's not associative in the above definition. If we select fb <*? fc first, we get a valid function that discards the invalid fc, so the result is equivalent to the invalid fa. If we select fa <*? fb first, the result is fa, and then combined with the invalid fc in the final step.

We can regain associativity by not combining two Invalids, but then ap != apS, where ap == apS is the paper's definition of rigidity:

val left: F[Either[A => B, B]] = ff.map(Left(_))
val right: F[(A => B) => B] = fa.map((a: A) => _(a))
left.select(right)

Copy link
Contributor

Choose a reason for hiding this comment

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

If you ask me, associativity is a must. The law ap == apS is one I would at least consider relaxing.

That law feels like it may preclude any rigid selects which are not also Monads.

And if you are a monad you are back to the part where we aren't really adding something new.

The struggle here feels like:

  1. Add something lawful
  2. Has non-trivial implementations that can't be implemented with Applicative
  3. Cannot be extended to Monad.

I'm not sure if I've seen an existence proof of something matching all three.

Copy link
Member Author

Choose a reason for hiding this comment

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

Several are close, but none hit the mark:

  • Const (aka Over) has no Monad and is rigid per the paper's definition: it passes the selective laws and ap == apS. It fails the our rigid skip laws.

  • There's FreeRigidSelective. It's ap == apS, but I think would also fail our rigid skip laws.

  • Validated and Under follow our skip laws, but not ap == apS.

  • All FlatMaps appear to pass ap == apS and selective associativity, but at least Map[K, *] and Tuple[X, *] fail our skip laws.

We may have conflated two concepts with what the paper calls rigid and our rigid skip laws, but everything that satisfies both seems to have a Monad.

fab match {
case Valid(Left(a)) => ff.map(_(a))
case Valid(Right(b)) => Valid(b)
case i @ Invalid(_) => i
}
}

private[data] class ValidatedApplicative[E: Semigroup] extends CommutativeApplicative[Validated[E, *]] {
override def map[A, B](fa: Validated[E, A])(f: A => B): Validated[E, B] =
fa.map(f)
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/scala/cats/instances/either.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package instances

import cats.data.Validated
import cats.kernel.Semigroup
import cats.syntax.EitherUtil
import cats.syntax.either._

import scala.annotation.tailrec
Expand Down Expand Up @@ -207,7 +206,7 @@ trait EitherInstances extends cats.kernel.instances.EitherInstances {
new Parallel[Either[E, *]] {
type F[x] = Validated[E, x]

def applicative: Applicative[Validated[E, *]] = Validated.catsDataApplicativeErrorForValidated
def applicative: Applicative[Validated[E, *]] = Validated.catsDataSelectiveErrorForValidated
def monad: Monad[Either[E, *]] = cats.instances.either.catsStdInstancesForEither

def sequential: Validated[E, *] ~> Either[E, *] =
Expand Down
1 change: 1 addition & 0 deletions core/src/main/scala/cats/syntax/all.scala
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ trait AllSyntax
with PartialOrderSyntax
with ProfunctorSyntax
with ReducibleSyntax
with SelectiveSyntax
with SemigroupSyntax
with SemigroupKSyntax
with ShowSyntax
Expand Down
Loading