-
Notifications
You must be signed in to change notification settings - Fork 603
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
Add Stream.evals, evalSeq #1582
Conversation
I can revert the changes scalafmt has made, if it'll make anything easier, but it's a separate commit anyway so should be easy for review. |
@@ -3068,6 +3141,14 @@ object Stream extends StreamLowPriority { | |||
def evalUnChunk[F[_], O](fo: F[Chunk[O]]): Stream[F, O] = | |||
fromFreeC(Algebra.eval(fo).flatMap(Algebra.output)) | |||
|
|||
/** Like `eval`, but lifts a foldable structure. **/ | |||
def evals[F[_], S[_]: Foldable, O](fo: F[S[O]]): Stream[F, O] = | |||
eval(fo).flatMap(_.foldMap(Stream.emit)) |
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.
I went with foldMap
, but also considered Stream.emits(foo.toList)
. Should I benchmark whether one would have a significant performance improvement over the other?
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.
Looks great! I like the idea of using Foldable and foldMap.
What about the Seq variant? Maybe I should use fromIterator in it. I'll give that a try and maybe do a quick benchmark. |
Okay cool -- I suspect the |
I'm going to merge as is for now, feel free to open a new PR if benchmark results change implementation. I'm getting the same Scalafmt results and want to avoid conflicts. |
The iterator solution is also massively slower than If anyone wants to optimize for a specific collection/size, they can always use the more low-level operators themselves. |
Closes #1571.