Add NonEmptyChain #2406
Add NonEmptyChain #2406
Conversation
|
Urgh, I'm apparently really bad at using git 🤦🏻♂️ |
|
|
||
|
|
||
| sealed class NonEmptyChainOps[A](val value: NonEmptyChain[A]) { | ||
|
|
| val iter: Iterator[AA] = toChain.iterator | ||
| var result = iter.next | ||
| while (iter.hasNext) { result = S.combine(result, iter.next) } | ||
| result |
There was a problem hiding this comment.
We should use combineAllOption.get so we don’t deoptimize combineAll
| @@ -76,6 +76,12 @@ object arbitrary extends ArbitraryInstances0 { | |||
| implicit def catsLawsCogenForNonEmptyList[A](implicit A: Cogen[A]): Cogen[NonEmptyList[A]] = | |||
| Cogen[List[A]].contramap(_.toList) | |||
|
|
|||
| implicit def catsLawsArbitraryForNonEmptyChain[A](implicit A: Arbitrary[A]): Arbitrary[NonEmptyChain[A]] = | |||
| Arbitrary(implicitly[Arbitrary[List[A]]].arbitrary.flatMap(fa => A.arbitrary.map(a => NonEmptyChain(a, fa: _*)))) | |||
|
|
|||
There was a problem hiding this comment.
Can we use the Arbitrary for Chain and add one more to the front or back also as a variant?
|
@LukaJCB it's not you are bad at using git, it's just the other PR is merged squashed. Since the changes look good, I think if we squash this one as well it will be okay. |
Codecov Report
@@ Coverage Diff @@
## master #2406 +/- ##
=========================================
+ Coverage 94.88% 95.2% +0.32%
=========================================
Files 350 351 +1
Lines 6258 6363 +105
Branches 284 285 +1
=========================================
+ Hits 5938 6058 +120
+ Misses 320 305 -15
Continue to review full report at Codecov.
|
| fa.toNonEmptyList | ||
| } | ||
|
|
||
| implicit def catsDataEqForNonEmptyChain[A: Eq]: Eq[NonEmptyChain[A]] = |
There was a problem hiding this comment.
we only have Eq here, no Order, PartialOrder, right? Can we add those so we get the priority right there?
9f0954f
to
e5b414c
Compare
| val iter = iterator | ||
| var i: Int = 0 | ||
| var i: Long = 0 |
There was a problem hiding this comment.
just curious, did we actually tried one instance with more than 2 billion items?
There was a problem hiding this comment.
No I don't think so, I just wanted to be consistent with NonEmptyList and NonEmptyVector :)
|
Merging with two signoffs :) |
* Add Catenable * Add law tests and simple arbitrary and eq instances * More tests * Add benchmarks * Add chain benchmarks * Add Iterator * More chain benchmarks * Add Paul and Pavel to Authors and change COPYING to Cats Contributors * More Tests * Add reverse, groupBy and zipWith * Add Collection Wrapping optimization * Add traverse and foldRight implementations that don't convert to List * Rename to Chain; add reverseIterator * More efficient implementations * Use Vector for reversing * Add NonEmptyChain * Add more fine grained operations to interop with Chain * Add NonEmptyChain * Use Chain For Arbitrary, don't deoptimize combineAll and syntax extends AnyVal * Add PartialOrder and Order * Add more tests * Even more tests
Based on the work in #2371, though we might still need to change a couple of things first, including the
ArbitraryGenerator.