Add EitherT.cond

[andyscott]

Adds EitherT.cond, which behaves just like Either.cond but lifted into the applicative functor F.

I thought about calling this fromBoolean, but I wanted to be consistent with the existing cond method on Either.

[codecov-io]

Current coverage is 91.94% (diff: 100%)

Merging #1481 into master will increase coverage by <.01%

@@             master      #1481   diff @@
==========================================
  Files           244        244          
  Lines          3621       3623     +2   
  Methods        3498       3502     +4   
  Messages          0          0          
  Branches        123        121     -2   
==========================================
+ Hits           3329       3331     +2   
  Misses          292        292          
  Partials          0          0          

Powered by Codecov. Last update 6c0cbe7...355b5a7

[kailuowang]

👍 thanks!

[peterneyens]

I think we need to import cats.data.EitherT and (for the Applicative[Future]) cats.implicits._, scala.concurrent.Future and an ExecutionContext. Newlines also need a | at the start of the line (see for example the toNested example).

It seems that this cond example wasn't tested by travis (if I haven't missed it), maybe because the format wasn't right ?

The easiest way I found to quickly test a doctest example is to paste it in a fresh console (otherwise it is easy to forget some of the imports).

Apart of the example 👍

[andyscott]

I had the doc example code running correctly, but the travis build was spitting out errors that the imports weren't needed. I actually went back and removed the imports just for the sake of appeasing Travis!

I'll push a new commit to see what happens. If it all goes well, I can squash before a merge.

[andyscott]

I pushed a new commit, but when I run the tests locally I get:

> test:compileIncremental
[info] Compiling 26 Scala sources to /Users/andy/git/cats/core/.jvm/target/scala-2.12/test-classes...
[error] /Users/andy/git/cats/core/.jvm/target/scala-2.12/src_managed/test/cats/data/EitherTDoctest.scala:217: Unused import
[error]     import cats.data.EitherT
[error]                      ^
[error] /Users/andy/git/cats/core/.jvm/target/scala-2.12/src_managed/test/cats/data/EitherTDoctest.scala:219: Unused import
[error]     import cats.implicits._
[error]                           ^
[error] /Users/andy/git/cats/core/.jvm/target/scala-2.12/src_managed/test/cats/data/EitherTDoctest.scala:221: Unused import
[error]     import scala.concurrent.Future
[error]                             ^
[error] /Users/andy/git/cats/core/.jvm/target/scala-2.12/src_managed/test/cats/data/EitherTDoctest.scala:223: Unused import
[error]     import scala.concurrent.ExecutionContext.Implicits.global
[error]                                                        ^
[error] four errors found
[error] (coreJVM/test:compileIncremental) Compilation failed
[error] Total time: 12 s, completed Dec 6, 2016 9:09:21 AM
>

[peterneyens]

I think the new line here is possibly the culprit.

This compiled for me :

scala> import cats.data.EitherT
scala> import cats.implicits._
scala> import scala.concurrent.Future
scala> import scala.concurrent.ExecutionContext.Implicits.global
scala> val userInput = "hello world"
scala> EitherT.cond[Future](
     |   userInput.forall(_.isDigit) && userInput.size == 10,
     |   userInput,
     |   "The input does not look like a phone number")
res0: EitherT[Future,String,String] = EitherT(Future(Success(Left(The input does not look like a phone number))))

Otherwise the generated test was :

include(new org.scalacheck.Properties("cond") {
  import cats.data.EitherT

  import cats.implicits._

  import scala.concurrent.Future

  import scala.concurrent.ExecutionContext.Implicits.global

  val userInput = "hello world"
})

Given how easy it seems for an example to not turn out exactly as we expect, I think the chances are pretty high that we currently have other examples/doctests which are not properly tested.

[peterneyens]

Thanks, sorry for the sbt-doctest / example issues you had to solve. 👍

[andyscott]

No worries. It really was just all spacing issues. And then there was another issue with Futures failing when the scalacheck tests ran for the doctest-- so I just switched to Id.