Add UnorderedFoldable and UnorderedTraverse - #1981
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1981 +/- ##
==========================================
- Coverage 95.25% 95.14% -0.11%
==========================================
Files 305 311 +6
Lines 5179 5252 +73
Branches 127 131 +4
==========================================
+ Hits 4933 4997 +64
- Misses 246 255 +9
Continue to review full report at Codecov.
|
81feb52 to
74b0228
Compare
|
I like the idea of introducing these two new type classes. |
|
Good question! I don't think there's a huge benefit apart from deduplication. |
|
You probably guessed what I would say next. They are useful but
Sorry, I am biased towards limiting scope in 1.0. |
|
Can we rebase master? I am curious if this one is bin compat with 1.0.0-RC1, Update: I can see benefits of having |
|
|
||
| /** | ||
| * `UnorderedTraverse` is like a `Traverse` for unordered containers. In addition to the traverse and sequence | ||
| * methods it provides nonEmptyTraverse and nonEmptySequence methods which require an `Apply` instance instead of `Applicative`. |
There was a problem hiding this comment.
This line of docs seems inaccurate?
| */ | ||
| def forall[A](fa: F[A])(p: A => Boolean): Boolean = | ||
| unorderedFoldMap(fa)(p)(UnorderedFoldable.andMonoid) | ||
| } |
There was a problem hiding this comment.
This could implement the size method from Foldable as well, I think.
| */ | ||
| def forallEmpty[A](fa: F[A], p: A => Boolean): Boolean = { | ||
| !F.isEmpty(fa) || F.forall(fa)(p) | ||
| } |
There was a problem hiding this comment.
Could it be made to be such that existsLazy and forallLazy are laws for UnorderedFoldable and not just Foldable?
There was a problem hiding this comment.
I think lazyness would require foldRight, no?
There was a problem hiding this comment.
I believe you can do it with Eval: see my duplicative attempt at this type class.
def exists[A](fa: F[A])(p: A => Boolean): Boolean =
foldMapUnordered(fa)(a => Eval.later(p(a)))(new CommutativeMonoid[Eval[Boolean]] {
def empty = Eval.False
def combine(x: Eval[Boolean], y: Eval[Boolean]): Eval[Boolean] =
x.flatMap(xv => if (xv) Eval.True else y)
}).valueand dually for forall.
There was a problem hiding this comment.
You're absolutely right, this is great! Thank you so much!
|
|
||
| import cats.data.Nested | ||
|
|
||
| trait UnorderedTraverseLaws[F[_]] extends UnorderedFoldableLaws[F] { |
There was a problem hiding this comment.
This could/should extend FunctorLaws[F] and assert some things about the compatibility of UnorderedTraverse and Functor; for instance, I would expect unorderedSequence to be unorderedTraverse on identity (which assumes that Eq for these instances respects unorderedness if it's not also a Traverse, I think)
There was a problem hiding this comment.
Hmmm, I'm not sure. That would imply that Set would be a valid Functor which I believe it is not. We could add a function like unorderedMap that's just unorderedTraverse with Id.
Thanks for your review by the way, it's been very helpful! :)
There was a problem hiding this comment.
No, you're right; I got a little bit overenthusiastic about drawing these parallels.
I think unorderedMap is kinda dubious, because the difference isn't even lack of order (which is unobservable in Functor#map already!), but that the operation can "drop" values, losing fusion, and therefore isn't a (categorical) Functor. I don't know what laws for unorderedMap would be, other than "it's the same as traversing with Id".
… into add-unordered-classes
5523596 to
f70fe6f
Compare
| } | ||
|
|
||
| def toSetRef[A](fa: F[A]): IsEq[Set[A]] = | ||
| F.unorderedFoldMap(fa)(a => Set(a)) <-> F.toSet(fa) |
There was a problem hiding this comment.
this could be
val lst1 = F.unorderedFoldMap(fa)(a => List(a))
val lst2 = F.toList(fa)
def contains[A: Eq](a: A): Boolean = lst1.exists(Eq[A].eqv(_, a))
val allCont = list2.map(contains)
val allTrue = lst.map(_ => true)
allCont <-> allTrueIn this way, we have a law that only needs Eq, not unusual, and the typeclass does not require any Order or Hash.
if we have an Order[A]. So we use an Order to check the law more quickly, or we could
… into add-unordered-classes
d0ac64e to
e2984b4
Compare
6 participants
As discussed in #1927, together with #1972 and #1966 this should fix #1831.
In this PR, I remove the
TraverseandFoldableinstances fromMapandSetand replace them withUnorderedTraverseandUnorderedFoldable. I think we could letFoldableandTraverseextend these.This PR depends on #1927.