-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Foldabe#dropWhile_ bug. #623
Conversation
In addition to fixing this bug, the commit defines several FoldableCheck tests for various types that have foldable instances. Finally, the commit adds takeWhile_. Fixes #563.
Current coverage is
|
package std | ||
|
||
trait IterableInstances { | ||
implicit val iterableInstance: Foldable[Iterable] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this belong in cats core? In a recent gitter conversation, the consensus was to put IndexedSeq
instances in alleycats because IndexedSeq
can potentially be mutable. Wouldn't the same arguments apply to Iterable
?
You didn't mention that this PR also adds a |
My claim is that foldable instances are different (e.g. we have one for |
@tpolecat what are your thoughts? Are we checking |
Bad Cop to the rescue! So, yeah I think val a = F.fold(xs)
val b = F.fold(xs) is not necessarily the same program as val a = F.fold(xs)
val b = a even though |
Ugh. OK, sure. Personally I'd prefer to use correctness-preserving when dealing with something like Scala-collections but I see your point. I'll remove it. 🔨 🔥 |
Iterable is one of the most useful Scala collection types. It's also impossible to be sure that its .iterator method doesn't side-effect. C'est la vie.
👍 |
1 similar comment
👍 |
In addition to fixing this bug, the commit defines several
FoldableCheck tests for various types that have foldable
instances.
Finally, the commit adds takeWhile_.
Fixes #563.