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

Modify XorT and OptionT variance #556

Closed
frosforever opened this Issue Oct 2, 2015 · 3 comments

Comments

Projects
None yet
3 participants
@frosforever
Copy link
Contributor

frosforever commented Oct 2, 2015

Xor is currently covariant while XorT is invariant.

Assuming one has the following:

sealed trait Error
case class ErrorA(message: String) extends Error
case object ErrorB extends Error

with Xor one can have something that looks like:

val t: Xor[Error, Int] = for {
  i <- Xor.left[ErrorB.type, Int](ErrorB)
  r <- Xor.left[ErrorA, Int](ErrorA("foo"))
} yield r

However the same using XorT will not compile:

val noCompile: XorT[List, Error, Int] = for {
  i <- XorT(List(Xor.left[ErrorB.type, Int](ErrorB)))
  r <- XorT(List(Xor.left[ErrorA, Int](ErrorA("foo"))))
} yield r

and one is instead forced to use

val tf: XorT[List, Error, Int] = for {
  i <- XorT(List(Xor.left[Error, Int](ErrorB)))
  r <- XorT(List(Xor.left[Error, Int](ErrorA("foo"))))
} yield r

Because Xor is covariant, the non-monad-transformer version of noCompile compiles just fine:

val compiles: List[Xor[Error, Int]] = List(Xor.left[ErrorB.type , Int](ErrorB)).flatMap { e1 =>
  List(Xor.left[ErrorA, Int](ErrorA("foo"))).map { e2 =>
    e1.flatMap { i =>
      e2.map { r =>
        r
      }
    }
  }
}
val compilesXorT = XorT[List, Error, Int](compiles)

I do not know which one is more correct but it is confusing for Xor and XorT not to behave in the same way. If this is indeed the expected and correct behavior perhaps we can add some documentation to prevent confusion.

Note: the same can be said for OptionT though I came across this using Xor and so these are the examples I've got.

@ceedubs

This comment has been minimized.

Copy link
Member

ceedubs commented Oct 3, 2015

Thanks for raising this issue and providing a detailed explanation @frosforever! I agree that we should do something about this, even if that "something" is just better documentation. Here is a bit of a brain dump on my thoughts on the matter.

In Scala, variance has both positives and negatives (perhaps that's how they decided on variance notation! :P). These pros/cons have been discussed numerous times in various venues, so I won't go into them now, but in my opinion at the end of the day you kind of have to settle on "to each their own".

I think this "to each their own" perspective implies that you can't force variance on a type constructor. A concrete example is scalaz.Free in the 7.0 series. It forces the S type constructor to be covariant. At the time I often wanted to wrap a Coyoneda in Free. The most recent versions of Cats and Scalaz have the Coyoneda essentially built into the Free, so this particular use might not be as desired now, but the general principle applies. The problem is that Coyoneda is invariant, so you simply couldn't do this (without a mess of @uncheckedVariance)! By making type constructor parameters invariant, you may end up forcing people to be more explicit about types, but I think that it beats the alternative, where you can prevent them from being able to use your type at all.

Does that make sense? Is there a best-of-both-worlds approach that I'm overlooking?

@zainab-ali

This comment has been minimized.

Copy link
Contributor

zainab-ali commented Jul 25, 2016

I prefer invariance for the reasons above, but still need a workaround to widen errors.
What do you think about adding a leftWiden on XorT? I'd be happy to make a PR for it.

@ceedubs

This comment has been minimized.

Copy link
Member

ceedubs commented Mar 24, 2018

I'm going to go ahead and close this out for a few reasons:

  • It's quiet for a long time.
  • As mentioned above, there are issues with forcing variance into type constructors that are used within EitherT.
  • Now that we have widen and leftWiden, this is easier to work around.

@ceedubs ceedubs closed this Mar 24, 2018

ceedubs added a commit to ceedubs/typelevel.github.com that referenced this issue Sep 30, 2018

Add a blog post aboutn variance and monad transformers
Every once in a while [people](typelevel/cats#556) [ask](typelevel/cats#2310) [about](typelevel/cats#2538) this.

I thought that it would be nice to provide a more detailed answer in a
single place so we can link to it from the Cats FAQ.

Note: I had to add the `-Ypartial-unification` flag for one of the code
samples to compile. It's quite possible that I should be doing this a
different way than I did.

ceedubs added a commit to ceedubs/typelevel.github.com that referenced this issue Sep 30, 2018

Add a blog post about variance and monad transformers
Every once in a while [people](typelevel/cats#556) [ask](typelevel/cats#2310) [about](typelevel/cats#2538) this.

I thought that it would be nice to provide a more detailed answer in a
single place so we can link to it from the Cats FAQ.

Note: I had to add the `-Ypartial-unification` flag for one of the code
samples to compile. It's quite possible that I should be doing this a
different way than I did.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment