Add foldMapK, reduceMapK

[kubukoz]

Closes #2275.

Not sure if we need to forward the target effect parameter of these functions in all the test classes, or whether we can just use Id or some other hardcoded type having a MonoidK, but for now I added appropriate implicits etc. everywhere.

[kubukoz]

BTW. we could also leave it as it is and use the G type instead of Id in the laws for foldM.

[kubukoz]

Some binary compat issues on 2.10/2.11 JVM, not sure how to solve

[kailuowang]

I am not sure about this one. It seems that a more generic approach would be provide an importable conversion from MonoidK to Monoid, something like

implicit def monoidKAlgebra[G[_], A])(implicit ev: MonoidK[G]): Monoid[G[A]] = ev.algebra

and then wherever a Monoid[G[A]] is needed you can just import this method. I am not sure how I feel about having a bunch of pairs of methods that one takes a Monoid and the other takes a MonoidK.

[kubukoz]

I don't know about others, but I personally don't like implicit functions that aren't converting to extension classes (which is a workaround for no value classes in traits/classes) :)

Edit: Wait, this is derivation (the argument is implicit) - so I guess it should be okay :D

What do others think?

[LukaJCB]

I think that particular method doesn't work when a type has different instances for Monoid and MonoidK. Maybe @tpolecat can chime in as he was the one with a use case for it :)

[kubukoz]

I just found another usecase for myself, WDYT about moving it to syntax for now (for bincompat) and continuing with it?

[LukaJCB]

Sounds good to me! :)

[ceedubs]

implicit def monoidKAlgebra[G[_], A])(implicit ev: MonoidK[G]): Monoid[G[A]] = ev.algebra

@kailuowang @kubukoz I don't think that we want to go down this path, as MonoidK and Monoid instances aren't always equivalent. For example in Cats the Option Monoid instance is based on combining with the inner Semigroup while the MonoidK instance is based on orElse. I think that there's a similar story for Function1/Endo but with function composition instead of orElse.

Edit: just to be clear, I was referring to a previous suggestion of deriving implicit Monoid instances based on MonoidK instances. I'm okay with the changes introduced by this PR, but I wonder about making them in a compatible way (or holding off until a scala 2.12+ release where the main changes are compatible). Is the extra law worth making an incompatible change to the laws?

[kailuowang]

It's a fair point that MonoidK and Monoid instance could be different. Let's proceed with adding this to bincompat syntax traits.

[LukaJCB]

@kubukoz Are you still working on this? :)

[kubukoz]

tbh I completely forgot about it. I think I'll start over (especially with all these conflicts) and get something out there soon :)

[codecov-io]

Codecov Report

Merging #2292 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2292      +/-   ##
==========================================
+ Coverage   95.12%   95.13%   +<.01%     
==========================================
  Files         365      365              
  Lines        6753     6757       +4     
  Branches      285      286       +1     
==========================================
+ Hits         6424     6428       +4     
  Misses        329      329

Impacted Files	Coverage Δ
core/src/main/scala/cats/syntax/reducible.scala	100% <100%> (ø)	⬆️
core/src/main/scala/cats/syntax/foldable.scala	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 342433c...e8e62dc. Read the comment docs.

[kubukoz]

@LukaJCB @kailuowang this one should be ready when the build is green :) sorry for the long wait

btw. the "Binary Breaking" label isn't relevant anymore

[kubukoz]

Rebased to resolve conflicts after #2671

[kubukoz]

Looks about green :) @LukaJCB

[LukaJCB]

Thanks!

[kubukoz]

ping @kailuowang

[kailuowang]

Lgtm, sorry about the delay