-
-
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
Add some Contravariant instances #590
Conversation
64f672e
to
cbe070b
Compare
The ContravariantTests don't work because of the Eq requirement. |
Current coverage is
|
@@ -21,6 +21,9 @@ final case class Kleisli[F[_], A, B](run: A => F[B]) { self => | |||
def map[C](f: B => C)(implicit F: Functor[F]): Kleisli[F, A, C] = | |||
Kleisli(a => F.map(run(a))(f)) | |||
|
|||
def contramap[C](f: C => B)(implicit F: Contravariant[F]): Kleisli[F, A, C] = | |||
Kleisli(a => F.contramap(run(a))(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.
I think the contramap
that I would expect for Kleisli
would be the local
method that already exists on it. I think in practice the F
in a Kleisli
is often going to be covariant (not contravariant) in nature (Future
, Task
, Xor
, etc), so I don't know how often this would be helpful. If you have some good examples of how you might use this, I'd definitely be interested in them.
Otherwise, should we get rid of this one and use local
for the Contravariant
instance?
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.
Yes, that does seem more useful. Is there a way to keep both the instances?
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.
For simplicity, I'd be inclined to just keep the one until someone runs into a compelling use-case for the other.
Thanks a bunch for working on this, @vikraman! |
For the missing implicit def kleisliEq[F[_], A:Arbitrary, B](implicit FB: Eq[F[B]]): Eq[Kleisli[F, A, B]] =
function1Eq[A, F[B]].on(_.run) |
This is an attempt to fix #579