-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
base: main
Are you sure you want to change the base?
WIP: Selective #3709
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Needs laws
- Needs tests
- Needs consideration of more efficient implementations for the monads
- Needs to be hammered in the community build
- Needs a review from someone with scars from our inheritance encoding
... but it passes MiMa so far!
I thought this would get stopped cold by binary compatibility and only expected to spend a few minutes on this. Alas, it seems to work. I'll pause here and see if anyone thinks this approach is fruitful. /cc @cb372 @hamishdickson whose prior art can be spliced in here if we proceed. /cc @LukaJCB @johnynek who both showed interest in the original. |
Codecov Report
@@ Coverage Diff @@
## master #3709 +/- ##
==========================================
- Coverage 90.24% 90.02% -0.22%
==========================================
Files 391 397 +6
Lines 8863 8975 +112
Branches 227 251 +24
==========================================
+ Hits 7998 8080 +82
- Misses 865 895 +30 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added my comments.
I'd love to see this move forward but I would suggest we not add Select at this point. If we are adding a select method that can be implemented with Apply I don't see any reason not to add it to Apply.
Here is an idea we can experiment with here: for every function on the typeclass we add a law that it behaves the same as the default. This allows us to override fearlessly and know that if the laws pass it must be correct. |
The Haskell implementation introduces an operator for |
new ValidatedSelective[E] with CommutativeApplicative[Validated[E, *]] | ||
|
||
@deprecated("Use catsDataCommutativeSelectiveForValidated", "2.4.0") | ||
def catsDataCommutativeApplicativeForValidated[E: CommutativeSemigroup]: CommutativeApplicative[Validated[E, *]] = |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I am a broken record now, so I don't need to keep commenting.
To summarize:
- I am strongly -1 on any typeclass which can lawfully be implemented by a superclass. In this case, it appears to me that Selective and Applicative do this. Just because Andrey did this in Haskell is not a sufficient argument, IMO. If we want select on Applicative, just put it there.
- I don't like to see us duplicate methods. Some of FlatMap/Monad methods seem to only require a RigidSelective (which I would call Selective and make all cats Selectives rigid, and if you want non-rigid, just use Applicative). I think we should move as many methods down as we can. I think ifM can be moved down, but I think others as well. Duplication confuses newcomers.
} | ||
|
||
@noop | ||
def ifS[A](fCond: F[Boolean])(fTrue: => F[A])(fFalse: => F[A]): F[A] = { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@@ -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] = |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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))
}
}
There was a problem hiding this comment.
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 Invalid
s, 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)
There was a problem hiding this comment.
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:
- Add something lawful
- Has non-trivial implementations that can't be implemented with Applicative
- Cannot be extended to Monad.
I'm not sure if I've seen an existence proof of something matching all three.
There was a problem hiding this comment.
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
(akaOver
) has noMonad
and is rigid per the paper's definition: it passes the selective laws andap == 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
andUnder
follow our skip laws, but notap == apS
. -
All
FlatMap
s appear to passap == apS
and selective associativity, but at leastMap[K, *]
andTuple[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
.
Nope, that last commit is bad. My solution was dubious, and I failed to test the rigid laws. |
@@ -186,6 +186,9 @@ import scala.annotation.implicitNotFound | |||
def whenA[A](cond: Boolean)(f: => F[A]): F[Unit] = | |||
if (cond) void(f) else unit | |||
|
|||
@noop | |||
def whenS[A](fCond: F[Boolean])(fTrue: => F[Unit]): F[Unit] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whenA
takes an F[A]
and voids it. whenS
should probably do the same.
core/src/main/scala/cats/Apply.scala
Outdated
def select[A, B](fab: F[Either[A, B]])(ff: => F[A => B]): F[B] = | ||
selectA(fab)(ff) | ||
|
||
def selectA[A, B](fab: F[Either[A, B]])(ff: F[A => B]): F[B] = |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
selectA
is likeifA
: requires 'Apply, not rigid even for
FlatMap`sselectM
(not added yet) is likeifM
: requiresFlatMap
, rigidselect
is likeifS
: 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.)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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:
- 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).
- 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).
- 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] {
...
}
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
andApplicative
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
:
- 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. - Depending how we resolve the above, some things may have a viable
ifM
without being aRigidSelective
, but we'd have no typeclass for these. We could pull those functions down toSelective
, but the name would lose its current sense of rigidity. - Some of the
'*M
functions are onFlatMap
. 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.
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 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.
Wanted to weigh in on this (a bit late, I know!): I'd like to point out that
Also I believe that there are some nice interactions between |
I'm unsure at this point whether it's easier to scrape the bitrot off this PR or start over and hope to not reopen the same issues that we did manage to work through here, but I'm interested in trying or handing the baton to someone else. |
somehow this PR popped into my head today, and it seemed to me like I found a solution. I propose the following:
I don't see that we proposed this law before, but I think it rules out an implementation using select(fab, fn) = map2(fab, fn) {
case (Right(b), _) => b
case (Left(a), f) => f(a)
} now consider
but using the definition above (map2-based) it will return I think this is what we want. Then we rule out For instance, I think def select[A, B](fab: Validated[E, Either[A, B]], fn: Validated[E, A => B]) =
fab match {
case Valid(Right(b)) =>
// do to Validated being pure and parametricity, this is the only value that can type check
Valid(b)
case Valid(Left(a)) =>
// do to Validated being pure and parametricity, this is the only value that can type check
fn.map(_(a))
case Invalid(e) =>
// we have to combine fn on error to satisfy the left/apply law
fn match {
case Valid(_) => Invalid(e)
case Invalid(e1) => Invalid(combine(e, e1))
}
} I think this is the only function that can match the above two laws. So, unless I'm mistaken, this solves the issues I had with Selective being the same as Applicative, and we don't need to add Selective and RigidSelective: we only add Selective (which is always rigid). Now, we could always add a function like |
Looking back at the original paper I don't think I actually made much progress here except to state rigid laws a bit more clearly in scala. I guess I still disagree with the paper and think Selective should be rigid and if you want to violate rigidity just make a Selective implemented with Applicative and don't check the Selective laws (just check the Applicative laws). |
If you want to go the rigid only direction, then perhaps the discussion here might be of interest: snowleopard/selective#74 I think one of the reasons this might not happen is because of supporting non-rigidity? |
To be fair, I think I'm on board with assuming rigidity, because otherwise, you can just implement the desired |
I also still don't see why a single use case stops the law which I think for many effects you want: you would almost certainly want to know that an effect isn't run unless required not that "maybe it will maybe it won't". If you want static analysis just make a new typeclass and instantiate it from Applicative or Selective. |
Also your argument about branch makes sense to me. |
One last point: if you want to do static analysis, can't you do it with a Free selective? The key issue is that there are no functions like bind/flatMap to evaluate in selective: nothing has the shape |
Experimental, incomplete integration of
Selective
into the hierarchy.See #2745