-
-
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
Kleisli contravariant on input type A #2843
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2843 +/- ##
=======================================
Coverage 94.21% 94.21%
=======================================
Files 368 368
Lines 6944 6944
Branches 301 301
=======================================
Hits 6542 6542
Misses 402 402
Continue to review full report at Codecov.
|
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 seems like a pretty reasonable change, given that we already have variance annotations for other data types in cats.data
, thanks a bunch for filing this PR :)
Interested in what other maintainers think
I am a little concerned with the source breaking widening, although the chance of them actually breaking on usesite is probably not very high. I would like to invite feedbacks from @rossabaker and @ChristopherDavenport on this one, given that |
I don't have a great intuition for inference problems. I'll try to publish a local version of cats and build http4s against that and look for trouble this week. |
Thanks a lot @rossabaker |
Not only should Kleisli be contravariant in It will most likely be source breaking, because in some cases users will have to annotate methods with the newly-added type parameter. |
@jdegoes I disagree, |
@LukaJCB Yes, I believe it's the correct decision to specialize for covariant functors. If anyone uses Kleisli with contravariant/invariant functors — and I haven't seen that in the wild — then this use case should be specialized as This is one of the many cases where embracing specialization results in unquestionably higher ergonomics at the loss of abstraction of questionable value that most users don't care about. |
What about revisiting the stance on covariance in monad transformers while we're at it? EDIT: Sorry, I did not update the page when submitting and did not see John's post which is more-or-less the same |
@Kaishh 👍 Major surgery required. |
Thanks @jdegoes, yeah I think as well it should be -A and +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.
This compiles with no changes on http4s-0.20.x when cherry-picked onto cats-1.6.0, under both Scala 2.11.12 and Scala 2.12.8.
Rationale: #2749
also as described in the test, it's more composable
scala can infer type to be
Kleisli[List, A1 with A2 with A3, (Int, String, Boolean)]
if not using scala subtyping it would looks like