Make cats.data.AndThen public #2297
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2297 +/- ##
==========================================
- Coverage 95.05% 95.03% -0.03%
==========================================
Files 338 338
Lines 5850 5878 +28
Branches 220 210 -10
==========================================
+ Hits 5561 5586 +25
- Misses 289 292 +3
Continue to review full report at Codecov.
|
| /** | ||
| * [[cats.Contravariant]] instance for [[AndThen]]. | ||
| */ | ||
| implicit def catsStdContravariantForAndThen[R]: Contravariant[AndThen[?, R]] = |
There was a problem hiding this comment.
Minor nitpick, but the standard convention is using the package name, so catsDataContravariantForAndThen :)
There was a problem hiding this comment.
Ah, I had no idea from where Std comes from 🙂
| /** | ||
| * [[cats.Monad]] instance for [[AndThen]]. | ||
| */ | ||
| implicit def catsStdMonadForAndThen[T]: Monad[AndThen[T, ?]] = |
| /** | ||
| * [[cats.ContravariantMonoidal]] instance for [[AndThen]]. | ||
| */ | ||
| implicit def catsStdContravariantMonoidalForAndThen[R : Monoid]: ContravariantMonoidal[AndThen[?, R]] = |
| * [[cats.arrow.CommutativeArrow CommutativeArrow]] instances | ||
| * for [[AndThen]]. | ||
| */ | ||
| implicit val catsStdArrowInstancesForAndThen: ArrowChoice[AndThen] with CommutativeArrow[AndThen] = |
There was a problem hiding this comment.
I agree that this is useful enough to release 👍 Thanks @alexandru, I left a couple of very minor comments regarding naming convention of implicits.
|
I renamed the instances. |
| @@ -124,3 +167,81 @@ private[cats] object AndThen { | |||
| */ | |||
| private final val fusionMaxStackDepth = 127 | |||
| } | |||
|
|
|||
| private[data] abstract class AndThenInstances1 extends AndThenInstances0 { | |||
There was a problem hiding this comment.
sorry to nitpick here, but the Cats conversion is to use 0 to represent the highest priority in implicit search. So we might want to switch the 0 and 1 here.
| checkAll("ContravariantMonoidal[AndThen[?, Int]]", SerializableTests.serializable(ContravariantMonoidal[AndThen[?, Int]])) | ||
| } | ||
|
|
||
| checkAll("AndThen[Int, Int]", MonadTests[Int => ?].monad[Int, Int, Int]) |
There was a problem hiding this comment.
This is not testing AndThen but Int => ?
| checkAll("AndThen[Int, Int]", ArrowChoiceTests[AndThen].arrowChoice[Int, Int, Int, Int, Int, Int]) | ||
| checkAll("ArrowChoice[AndThen]", SerializableTests.serializable(ArrowChoice[AndThen])) | ||
|
|
||
| checkAll("AndThen[Int, Int]", ContravariantTests[? => Int].contravariant[Int, Int, Int]) |
There was a problem hiding this comment.
Same here. Not testing AndThen
| /** | ||
| * [[cats.Contravariant]] instance for [[AndThen]]. | ||
| */ | ||
| implicit def catsContravariantForAndThen[R]: Contravariant[AndThen[?, R]] = |
There was a problem hiding this comment.
sorry another nitpick: the Cats naming convention for implicits requires the package name to be included as well. So these should be catsDataContravariantForAndThen, but since they are in the companion object it doesn't matter much, just to be consistent.
|
@kailuowang I fixed your comments. |
I'm increasingly finding use-cases for
AndThenand it is frustrating that I have to copy it around.Now I need it in Monix, to work with
Iterant's new encoding. And I remember another use-case for it in cats-effect. As explained in the ScalaDoc, it's an incredibly useful data type for whenever you have to work withA => F[B].In such cases you can piggyback on
F[_]if it implements a stack-safeflatMap, howeverF[_]can be more expensive than usage ofAndThen.I'd like
AndThento be incats-core, because we need it forIndexedStateTanyway and because it is reusable.N.B. this doesn't break binary compatibility and so it could be released in the 1.x series.