Add Semigroup and Monoid combinators reverse and intercalate

[johnynek]

This adds a few things:

1. reverse method on Semigroup which returns a new Semigroup with the order reversed
2. intercalate method on Semigroup which returns a new Semigroup that puts an element between each combine operation
3. some optimizations of default implementations using laws.
4. reduce the width of the duration Arbitraries so we don't overflow (which causes exceptions).

[non]

Looks like some annoying 2.13 deprecation warnings. But once those are cleaned up I'm 👍 on this!

[johnynek]

someone suggested renaming intercalate to intersperse, which I think it actually a better name.

I'll update to that. @LukaJCB @non what do you think?

[johnynek]

actually, Foldable already uses intercalate for this, so I guess we should stick with that.

[johnynek]

also @kubukoz correctly pointed out if we test associativity of combine, we don't need to test associativity of reverse.combine or intercalate.combine after we check that they are actually reversing or intercalating because the fact they are associative is true for all associative combines:

val rev = semi.reverse
rev.combine(a, rev.combine(b, c)) =
  combine(combine(c, b), a) =
  combine(c, combine(b, a)) =
  rev.combine(rev.combine(a, b), c)

val intr = semi.intercalate(m)
// using <+> for intr and + for semi:
a <+> (b <+> c) =
  a + m + (b + m + c) =
  (a + m + b) + m + c =
  (a <+> b) <+> c

so I removed those associativity checks after we check that reverse and intercalate are actually doing reverse and intercalate.

[codecov-io]

Codecov Report

Merging #3279 into master will decrease coverage by 0.07%.
The diff coverage is 98.21%.

@@            Coverage Diff             @@
##           master    #3279      +/-   ##
==========================================
- Coverage   93.14%   93.07%   -0.08%     
==========================================
  Files         378      378              
  Lines        7576     7628      +52     
  Branches      203      206       +3     
==========================================
+ Hits         7057     7100      +43     
- Misses        519      528       +9

Flag	Coverage Δ
#scala_version_212	93.32% <98.21%> (-0.08%)	⬇️
#scala_version_213	92.85% <98.21%> (-0.07%)	⬇️

Impacted Files	Coverage Δ
...src/main/scala/cats/kernel/CommutativeMonoid.scala	0% <ø> (ø)	⬆️
core/src/main/scala/cats/Reducible.scala	100% <100%> (ø)	⬆️
...rc/main/scala/cats/kernel/laws/SemigroupLaws.scala	100% <100%> (ø)	⬆️
...rc/main/scala/cats/kernel/BoundedSemilattice.scala	60% <100%> (+60%)	⬆️
core/src/main/scala/cats/Foldable.scala	92.59% <100%> (-5.92%)	⬇️
...a/cats/kernel/laws/discipline/SemigroupTests.scala	100% <100%> (ø)	⬆️
kernel/src/main/scala/cats/kernel/Monoid.scala	80% <100%> (+7.27%)	⬆️
kernel/src/main/scala/cats/kernel/Semigroup.scala	77.27% <100%> (+6.68%)	⬆️
...in/scala/cats/kernel/instances/ListInstances.scala	100% <100%> (ø)	⬆️
.../main/scala/cats/kernel/CommutativeSemigroup.scala	50% <100%> (+50%)	⬆️
... and 4 more

---

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 bb7a180...86a5088. Read the comment docs.

[johnynek]

looks like BigDecimal isn't really commutative in 2.13

[info] - CommutativeGroup[BigDecimal].commutativeGroup.intercalateRepeat2 *** FAILED ***

[info]   GeneratorDrivenPropertyCheckFailedException was thrown during property evaluation.

[info]    (LawTests.scala:321)

[info]     Falsified after 82 successful property evaluations.

[info]     Location: (LawTests.scala:321)

[info]     Occurred when passed generated values (

[info]       arg0 = 3.1935566048388094E+154,

[info]       arg1 = 1.1175491601257495E+136

[info]     )

[info]     Label of failing property:

[info]       Expected: 3.193556604838809402235098320251500E+154

[info]   Received: 3.193556604838809402235098320251499E+154

[djspiewak]

Minor nit, but why not just use this?

[travisbrown]

@johnynek What do you think about reverting 86a5088 now that #3303 is in? The optimization is valid and the BigDecimal tests won't fail now.

[travisbrown]

I think we can go ahead and merge this now and reinstate the intercalate optimization in a follow-up if that's agreeable to everyone.

[johnynek]

I never know who has authority to merge things here. It seems people approve but don’t merge. I think it creates a bad experience for contributors. I think technically I have the merge bit so I could have clicked merge but culturally I think active maintainers should be the ones making the calls wrt release schedules etc...

[travisbrown]

@johnynek I was planning to merge but wanted to give you a day or two to follow up on the question about the intercalate change, if you wanted to. I'll go ahead and merge now and we can do that later.

[travisbrown]

@johnynek (For the record though I don't think anyone would have looked sideways if you'd merged it yourself after the second checkmark—I definitely wouldn't have.)