Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add Foldable, Traverse and Comonad instances to WriterT #2182

Merged
merged 16 commits into from
Mar 12, 2018
Merged

Add Foldable, Traverse and Comonad instances to WriterT #2182

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0f3b16c
Added foldLeft and foldRight to WriterT and Foldable instance
barambani Mar 3, 2018
4d1d7f7
Added tests for Foldable instance
barambani Mar 3, 2018
241f213
Added traverse. Fixed priority of the implicit search for isntances
barambani Mar 4, 2018
e68849d
Added traverse implementation to WriterT
barambani Mar 4, 2018
6729608
Added test for WriterT Traverse
barambani Mar 4, 2018
2a7f5f2
Refactored names
barambani Mar 5, 2018
454cc8f
Changed priority of Traverse instance
barambani Mar 5, 2018
4cf8304
Added comonad instance for WrtiterT and Const. Added tests
barambani Mar 7, 2018
0c2c002
Completed Comonad tests using tuple instead of Const
barambani Mar 7, 2018
e63a7e3
Refactored extract derivation to avoid implicit search
barambani Mar 7, 2018
fba0f7a
Added instances and reolution tests for Id. Removed unneeded type
barambani Mar 9, 2018
2e4b2b5
Minor refactoring
barambani Mar 9, 2018
fd077c3
Attempt to increase coverage
barambani Mar 9, 2018
a40e9f7
Removed unneeded serialization test
barambani Mar 9, 2018
b3ab664
Refactored traverse derivation
barambani Mar 12, 2018
152ee7a
Fixed improper inheritance of WriterTFunctor
barambani Mar 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 73 additions & 12 deletions core/src/main/scala/cats/data/WriterT.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package cats
package data

import cats.Foldable
import cats.kernel.instances.tuple._

import cats.kernel.CommutativeMonoid
import cats.syntax.semigroup._

Expand Down Expand Up @@ -63,6 +63,17 @@ final case class WriterT[F[_], L, V](run: F[(L, V)]) {
mapWritten(_ => monoidL.empty)

def show(implicit F: Show[F[(L, V)]]): String = F.show(run)

def foldLeft[C](c: C)(f: (C, V) => C)(implicit F: Foldable[F]): C =
F.foldLeft(run, c)((z, lv) => f(z, lv._2))

def foldRight[C](lc: Eval[C])(f: (V, Eval[C]) => Eval[C])(implicit F: Foldable[F]): Eval[C] =
F.foldRight(run, lc)((lv, z) => f(lv._2, z))

def traverse[G[_], V1](f: V => G[V1])(implicit F: Traverse[F], G: Applicative[G]): G[WriterT[F, L, V1]] =
G.map(
F.traverse(run)(lv => G.tupleLeft(f(lv._2), lv._1))
)(WriterT.apply)
}

object WriterT extends WriterTInstances with WriterTFunctions {
Expand Down Expand Up @@ -95,9 +106,23 @@ private[data] sealed abstract class WriterTInstances extends WriterTInstances0 {
implicit val F0: Monad[F] = F
implicit val L0: Monoid[L] = L
}

implicit def catsDataTraverseForWriterTId[L](implicit F: Traverse[Id]): Traverse[WriterT[Id, L, ?]] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@barambani you can't have any instance that potentially conflicts in the same trait level.
I think there is Functor instance conflict here for [WriterT[Id, L, ?]. I think you might need to create quite a few more priority layers to separate the instances.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be clear I didn't see the actually error message on travis. But I think separating them out might solve the problem.

Copy link
Contributor Author

@barambani barambani Mar 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the confusion, my bad for the lack of clarity. This prioritisation should work (let's see when the CI completes). My question was more in general about the reason why we needed to create instances for Id. I would have expected that something like Comonad[WriterT[Id, ListWrapper[Int], ?]] could summon the instance for F[_] and I was wondering if you had resources about why that's not the case.
This doesn't mean that I'm not happy to add more priority levels if you think that's better. Thanks for the great help here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@barambani I don't know the underlying reason that Id needs to be special-cased, but it's a weakness in scalac's type inference. This is a related Stack Overflow post that I've seen before. Something like Id might be a tricky one because A =:= Id[A] =:= Id[Id[A]], so there may be some logic to short-circuit inference when applying a type results in the same type so you don't end up in an infinite loop or something. This is pure speculation :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ceedubs thanks a lot, I didn't want for you to waste time on this. I was just taking advantage of this discussion in case you knew it out of your head 😃 👍

catsDataTraverseForWriterT[Id, L](F)
}

private[data] sealed abstract class WriterTInstances0 extends WriterTInstances1 {

implicit def catsDataTraverseForWriterT[F[_], L](implicit F: Traverse[F]): Traverse[WriterT[F, L, ?]] =
new WriterTTraverse[F, L] {
val F0: Traverse[F] = F
}

implicit def catsDataFoldableForWriterTId[L](implicit F: Foldable[Id]): Foldable[WriterT[Id, L, ?]] =
catsDataFoldableForWriterT[Id, L](F)
}

private[data] sealed abstract class WriterTInstances1 extends WriterTInstances2 {
implicit def catsDataMonadErrorForWriterT[F[_], L, E](implicit F: MonadError[F, E], L: Monoid[L]): MonadError[WriterT[F, L, ?], E] =
new WriterTMonadError[F, L, E] {
implicit val F0: MonadError[F, E] = F
Expand Down Expand Up @@ -134,9 +159,14 @@ private[data] sealed abstract class WriterTInstances0 extends WriterTInstances1

implicit def catsDataMonoidForWriterTId[L:Monoid, V:Monoid]: Monoid[WriterT[Id, L, V]] =
catsDataMonoidForWriterT[Id, L, V]

implicit def catsDataFoldableForWriterT[F[_], L](implicit F: Foldable[F]): Foldable[WriterT[F, L, ?]] =
new WriterTFoldable[F, L]{
val F0: Foldable[F] = F
}
}

private[data] sealed abstract class WriterTInstances1 extends WriterTInstances2 {
private[data] sealed abstract class WriterTInstances2 extends WriterTInstances3 {
implicit def catsDataMonadForWriterTId[L:Monoid]: Monad[WriterT[Id, L, ?]] =
catsDataMonadForWriterT[Id, L]

Expand All @@ -145,9 +175,12 @@ private[data] sealed abstract class WriterTInstances1 extends WriterTInstances2

implicit def catsDataSemigroupForWriterTId[L:Semigroup, V:Semigroup]: Semigroup[WriterT[Id, L, V]] =
catsDataSemigroupForWriterT[Id, L, V]

implicit def catsDataComonadForWriterTId[L](implicit F: Comonad[Id]): Comonad[WriterT[Id, L, ?]] =
catsDataComonadForWriterT[Id, L](F)
}

private[data] sealed abstract class WriterTInstances2 extends WriterTInstances3 {
private[data] sealed abstract class WriterTInstances3 extends WriterTInstances4 {
implicit def catsDataMonadForWriterT[F[_], L](implicit F: Monad[F], L: Monoid[L]): Monad[WriterT[F, L, ?]] =
new WriterTMonad[F, L] {
implicit val F0: Monad[F] = F
Expand All @@ -163,12 +196,12 @@ private[data] sealed abstract class WriterTInstances2 extends WriterTInstances3
catsDataCoflatMapForWriterT[Id, L]
}

private[data] sealed abstract class WriterTInstances3 extends WriterTInstances4 {
private[data] sealed abstract class WriterTInstances4 extends WriterTInstances5 {
implicit def catsDataFlatMapForWriterTId[L:Semigroup]: FlatMap[WriterT[Id, L, ?]] =
catsDataFlatMapForWriterT2[Id, L]
}

private[data] sealed abstract class WriterTInstances4 extends WriterTInstances5 {
private[data] sealed abstract class WriterTInstances5 extends WriterTInstances6 {
implicit def catsDataFlatMapForWriterT1[F[_], L](implicit F: FlatMap[F], L: Monoid[L]): FlatMap[WriterT[F, L, ?]] =
new WriterTFlatMap1[F, L] {
implicit val F0: FlatMap[F] = F
Expand All @@ -181,15 +214,15 @@ private[data] sealed abstract class WriterTInstances4 extends WriterTInstances5
}
}

private[data] sealed abstract class WriterTInstances5 extends WriterTInstances6 {
private[data] sealed abstract class WriterTInstances6 extends WriterTInstances7 {
implicit def catsDataApplicativeErrorForWriterT[F[_], L, E](implicit F: ApplicativeError[F, E], L: Monoid[L]): ApplicativeError[WriterT[F, L, ?], E] =
new WriterTApplicativeError[F, L, E] {
implicit val F0: ApplicativeError[F, E] = F
implicit val L0: Monoid[L] = L
}
}

private[data] sealed abstract class WriterTInstances6 extends WriterTInstances7 {
private[data] sealed abstract class WriterTInstances7 extends WriterTInstances8 {
implicit def catsDataAlternativeForWriterT[F[_], L](implicit F: Alternative[F], L: Monoid[L]): Alternative[WriterT[F, L, ?]] =
new WriterTAlternative[F, L] {
implicit val F0: Alternative[F] = F
Expand All @@ -202,7 +235,7 @@ private[data] sealed abstract class WriterTInstances6 extends WriterTInstances7
}
}

private[data] sealed abstract class WriterTInstances7 extends WriterTInstances8 {
private[data] sealed abstract class WriterTInstances8 extends WriterTInstances9 {
implicit def catsDataMonoidKForWriterT[F[_], L](implicit F: MonoidK[F]): MonoidK[WriterT[F, L, ?]] =
new WriterTMonoidK[F, L] {
implicit val F0: MonoidK[F] = F
Expand All @@ -219,7 +252,7 @@ private[data] sealed abstract class WriterTInstances7 extends WriterTInstances8
}
}

private[data] sealed abstract class WriterTInstances8 extends WriterTInstances9 {
private[data] sealed abstract class WriterTInstances9 extends WriterTInstances10 {
implicit def catsDataSemigroupKForWriterT[F[_], L](implicit F: SemigroupK[F]): SemigroupK[WriterT[F, L, ?]] =
new WriterTSemigroupK[F, L] {
implicit val F0: SemigroupK[F] = F
Expand All @@ -230,18 +263,24 @@ private[data] sealed abstract class WriterTInstances8 extends WriterTInstances9
implicit val F0: Applicative[F] = F
implicit val L0: Monoid[L] = L
}

}

private[data] sealed abstract class WriterTInstances9 extends WriterTInstances10 {
private[data] sealed abstract class WriterTInstances10 extends WriterTInstances11 {
implicit def catsDataApplyForWriterT[F[_], L](implicit F: Apply[F], L: Semigroup[L]): Apply[WriterT[F, L, ?]] =
new WriterTApply[F, L] {
implicit val F0: Apply[F] = F
implicit val L0: Semigroup[L] = L
}
}

private[data] sealed abstract class WriterTInstances10 {
private[data] sealed abstract class WriterTInstances11 extends WriterTInstances12 {
implicit def catsDataComonadForWriterT[F[_], L](implicit F: Comonad[F]): Comonad[WriterT[F, L, ?]] =
new WriterTComonad[F, L] {
implicit val F0: Comonad[F] = F
}
}

private[data] sealed abstract class WriterTInstances12 {
implicit def catsDataCoflatMapForWriterT[F[_], L](implicit F: Functor[F]): CoflatMap[WriterT[F, L, ?]] =
new WriterTCoflatMap[F, L] {
implicit val F0: Functor[F] = F
Expand Down Expand Up @@ -410,6 +449,28 @@ private[data] sealed trait WriterTCoflatMap[F[_], L] extends CoflatMap[WriterT[F
def coflatMap[A, B](fa: WriterT[F, L, A])(f: WriterT[F, L, A] => B): WriterT[F, L, B] = fa.map(_ => f(fa))
}

private[data] sealed trait WriterTFoldable[F[_], L] extends Foldable[WriterT[F, L, ?]] {

implicit def F0: Foldable[F]

def foldLeft[A, B](fa: WriterT[F, L, A], b: B)(f: (B, A) => B): B = fa.foldLeft(b)(f)
def foldRight[A, B](fa: WriterT[F, L, A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = fa.foldRight(lb)(f)
}

private[data] sealed trait WriterTTraverse[F[_], L] extends Traverse[WriterT[F, L, ?]] with WriterTFoldable[F, L] with WriterTFunctor[F, L] {

override implicit def F0: Traverse[F]

override def map[A, B](fa: WriterT[F, L, A])(f: A => B): WriterT[F, L, B] = super[WriterTFunctor].map(fa)(f)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can omit this line if we defined map in WriterTFunctor as override def map.

See for example EitherTFunctor and EitherTMonad (although EitherTTraverse doesn't extends EitherTFunctor at the moment and neither does OptionTTraverse OptionTFunctor).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very true, apologies. I'm fixing it. I rushed it in. I have to stop reading those intellij errors. Do you think I should change also EitherTTraverse and OptionTTraverse accordingly ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are probably more cases than just EitherTTraverse and OptionTTraverse which can be improved, I would leave those for another PR.

def traverse[G[_]: Applicative, A, B](fa: WriterT[F, L, A])(f: A => G[B]): G[WriterT[F, L, B]] = fa.traverse(f)
}

private[data] sealed trait WriterTComonad[F[_], L] extends Comonad[WriterT[F, L, ?]] with WriterTCoflatMap[F, L] {

override implicit def F0: Comonad[F]

def extract[A](fa: WriterT[F, L, A]): A = F0.extract(F0.map(fa.run)(_._2))
}

private[data] trait WriterTFunctions {
def putT[F[_], L, V](vf: F[V])(l: L)(implicit functorF: Functor[F]): WriterT[F, L, V] =
Expand Down
48 changes: 48 additions & 0 deletions tests/src/test/scala/cats/tests/WriterTSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,54 @@ class WriterTSuite extends CatsSuite {
checkAll("ContravariantMonoidal[WriterT[Const[String, ?], Int, ?]]", SerializableTests.serializable(ContravariantMonoidal[WriterT[Const[String, ?], Int, ?]]))
}

{
// F has a Foldable and L has a Monoid
implicit val L: Monoid[ListWrapper[Int]] = ListWrapper.monoid[Int]
Foldable[Const[String, ?]]
Foldable[WriterT[Const[String, ?], ListWrapper[Int], ?]]

checkAll("WriterT[Const[String, ?], ListWrapper[Int], ?]", FoldableTests[WriterT[Const[String, ?], ListWrapper[Int], ?]].foldable[Int, Int])
checkAll("Foldable[WriterT[Const[String, ?], ListWrapper[Int], ?]]", SerializableTests.serializable(Foldable[WriterT[Const[String, ?], ListWrapper[Int], ?]]))

Foldable[Id]
Foldable[WriterT[Id, ListWrapper[Int], ?]]
Foldable[Writer[ListWrapper[Int], ?]]

checkAll("WriterT[Id, ListWrapper[Int], ?]", FoldableTests[WriterT[Id, ListWrapper[Int], ?]].foldable[Int, Int])
}

{
// F has a Traverse and L has a Monoid
implicit val L: Monoid[ListWrapper[Int]] = ListWrapper.monoid[Int]
Traverse[Const[String, ?]]
Traverse[WriterT[Const[String, ?], ListWrapper[Int], ?]]

checkAll("WriterT[Const[String, ?], ListWrapper[Int], ?]", TraverseTests[WriterT[Const[String, ?], ListWrapper[Int], ?]].traverse[Int, Int, Int, Int, Option, Option])
checkAll("Traverse[WriterT[Const[String, ?], ListWrapper[Int], ?]]", SerializableTests.serializable(Traverse[WriterT[Const[String, ?], ListWrapper[Int], ?]]))

Traverse[Id]
Traverse[WriterT[Id, ListWrapper[Int], ?]]
Traverse[Writer[ListWrapper[Int], ?]]

checkAll("WriterT[Id, ListWrapper[Int], ?]", TraverseTests[WriterT[Id, ListWrapper[Int], ?]].traverse[Int, Int, Int, Int, Option, Option])
}

{
// F has a Comonad and L has a Monoid
implicit val L: Monoid[ListWrapper[Int]] = ListWrapper.monoid[Int]
Comonad[(String, ?)]
Comonad[WriterT[(String, ?), ListWrapper[Int], ?]]

checkAll("WriterT[(String, ?), ListWrapper[Int], ?]", ComonadTests[WriterT[(String, ?), ListWrapper[Int], ?]].comonad[Int, Int, Int])
checkAll("Comonad[WriterT[(String, ?), ListWrapper[Int], ?]]", SerializableTests.serializable(Comonad[WriterT[(String, ?), ListWrapper[Int], ?]]))

Comonad[Id]
Comonad[WriterT[Id, ListWrapper[Int], ?]]
Comonad[Writer[ListWrapper[Int], ?]]

checkAll("WriterT[Id, ListWrapper[Int], ?]", ComonadTests[WriterT[Id, ListWrapper[Int], ?]].comonad[Int, Int, Int])
}

checkAll("WriterT[Option, Int, ?]", CommutativeMonadTests[WriterT[Option, Int, ?]].commutativeMonad[Int, Int, Int])
checkAll("CommutativeMonad[WriterT[Option, Int, ?]]",SerializableTests.serializable(CommutativeMonad[WriterT[Option, Int, ?]]))
}