#500 add derived foldMap from traverse law

[julien-truffaut]

I probably missed something in the definition of the law because they all pass but a simple test shows that it is not the case for List

[julien-truffaut]

@non could it be related to the limitation of Arbitrary function you mentioned recently?

[julien-truffaut]

Traverse laws still pass with non commutative Monoid. If I understand correctly it is because because the arbitrary instance of Function only generates constant function

[non]

@julien-truffaut Correct. Once we have a new ScalaCheck release we will start getting real functions which actually depend on their input.

[julien-truffaut]

@non so what do you recommend to do?

• commit new law (remove unit test in List) and wait for scalacheck upgrade to fix instances
• commit new law and try to fix as many instances as possible
• something else?

[non]

@julien-truffaut I guess I would say we should do (1). To me that seems like the best path, and will avoid duplicate (or redundant) work. But I'm fine with any of these to be honest.

[julien-truffaut]

I went for (2) but only List, Vector and Stream (I need them for monocle test to pass).

I could fix List and Vector, we can probably have more efficient impl for Vector. However, I am struggling with Stream, I wouldn't mind if someone else would like to give it a try.

[non]

OK, I'll take a look.

[ceedubs]

@julien-truffaut I was perplexed by the discrepancy between the traverse/const and foldMap for List, and I realized that our implementation of ap for Const is the reverse of what is in scalaz. Are you aware of that? At first glance, both implementations appear to satisfy the necessary laws to me, but I could definitely be mistaken.

Cats:

scala> val x = Const[List[Int], String](List(1)) ap Const[List[Int], String => String](List(2))
x: cats.data.Const[List[Int],String] = Const(List(1, 2))

Scalaz:

scala> val x = Const[List[Int], String](List(1)) <*> Const[List[Int], String => String](List(2))
x: scalaz.Const[List[Int],String] = Const(List(2, 1))

[non]

@ceedubs I also noticed some interesting order issues in the monoidal branch I'm working on.

[aryairani]

flashback to #142

[julien-truffaut]

@ceedubs no I didn't notice. It seems that haskell use the same order than cats http://hackage.haskell.org/package/base-4.8.1.0/docs/src/Control.Applicative.html#line-84

[ceedubs]

@julien-truffaut I think Haskell's is actually the opposite of Cats. It looks the same because the value from the first const is "prepended" to the value from the second const. However, the function argument comes first in Haskell while it comes second in Cats. I think that it makes sense to have the function come first in Haskell but last in Scala, due to differences in the languages. However, it does mean that the version of Const.ap that we would probably want for traverse would probably seem backwards if used by itself. I suspect this might become a non-issue if @non's monoidal work replaces ap (though I'm not sure - the plan might be to leave it in).

[julien-truffaut]

@ceedubs maybe I missed something but it seems to me that haskell behave in the same way than cats

Prelude Control.Applicative Data.Monoid> Const(3 : []) <*> Const(5 : [])
Const [3,5]

[julien-truffaut]

some additional background of scalaz impl: scalaz/scalaz@084e9c9#commitcomment-13182666

[julien-truffaut]

@ceedubs found out exactly what was the issue!

[codecov-io]

Current coverage is 65.09%

Merging #502 into master will increase coverage by +0.01% as of 01d807b

@@            master    #502   diff @@
======================================
  Files          156     156       
  Stmts         2417    2418     +1
  Branches        68      68       
  Methods          0       0       
======================================
+ Hit           1573    1574     +1
  Partial          0       0       
  Missed         844     844       

Review entire Coverage Diff as of 01d807b

Powered by Codecov. Updated on successful CI builds.

[non]

👍

[non]

Sorry for the new merge conflicts. If this gets updated I'm happy to see it merged. Thanks!

[adelbertc]

👍 from me once merge conflicts resolved (and Travis is green light)

[ceedubs]

What @adelbertc said ^