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.

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

Adding CoflatMap and tests to WriterT and fixed laws for CoflatMap #1049

Merged
merged 4 commits into from
May 23, 2016

Conversation

zainab-ali
Copy link
Contributor

Adding a CoflatMap instance to WriterT, as required in #620
Added all laws in CoflatMapLaws to CoflatMapTests

implicit val F: Functor[ListWrapper] = ListWrapper.functor

CoflatMap[WriterT[ListWrapper, Int, ?]]
checkAll("WriterT[Listwrapper, Int, ?]", CoflatMapTests[WriterT[ListWrapper, Int, ?]].coflatMap[Int, Int, Int])
Copy link
Contributor

Choose a reason for hiding this comment

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

need to check serializability

@@ -87,8 +87,7 @@ private[data] sealed abstract class WriterTInstances0 extends WriterTInstances1
implicit val L0: Monoid[L] = L
}

implicit def writerTIdFunctor[L]: Functor[WriterT[Id, L, ?]] =
writerTFunctor[Id, L]
def writerTIdFunctor[L]: Functor[WriterT[Id, L, ?]] = writerTIdCoflatMap
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to delete this now that writerTIdCoflatMap exists. You might want to move it up to where this was.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I move it up, the implicit conflicts with writerTIdFlatMap when there is a Semigroup for L, as they are both Functors

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see. Glad we have tests for that! :). Do things work if you just remove this writerTIdFunctor?

@ceedubs
Copy link
Contributor

ceedubs commented May 20, 2016

Thanks @zainab-ali! This is definitely a nice change. I think it allows us to remove a couple Functor instances and have left some inline comments in the relevant places.

@@ -168,7 +171,9 @@ private[data] sealed abstract class WriterTInstances6 extends WriterTInstances7
}

private[data] sealed abstract class WriterTInstances7 {
implicit def writerTFunctor[F[_], L](implicit F: Functor[F]): Functor[WriterT[F, L, ?]] = new WriterTFunctor[F, L] {

implicit def writerTCoflatMap[F[_], L](implicit F: Functor[F]): WriterTCoflatMap[F, L] =
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the return type here should be CoflatMap[WriterT[F, L, ?]]

@ceedubs
Copy link
Contributor

ceedubs commented May 22, 2016

@zainab-ali looks great! Thanks again. 👍

@adelbertc
Copy link
Contributor

LGTM 👍

@adelbertc adelbertc merged commit 8fefdff into typelevel:master May 23, 2016
@ceedubs ceedubs mentioned this pull request Jun 2, 2016
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants