Interleave effects better in chunksOf

[treeowl]

Generalize the type of chunksOf to allow the inner and outer
streams of the result to use different underlying monads. This
forces effects to be handled within the inner monad, leading
to an arguably better sort of chunking.

Fixes #45

[treeowl]

@andrewthad, do you think you could weigh in on this? Is there any reason to prefer the old semantics? Is there any potential performance gotcha here?

[treeowl]

Ah, I see the weakness now, @torgeirsh. As far as I can tell, this version will/can turn the empty stream into a stream producing the empty stream, instead of just an empty stream. I have the feeling there might also be ways to get evenly divided streams to end up with empty end bits. I suspect users will be surprised by this. The trouble is that this commits to producing a functor layer before knowing if the stream is finished.

[andrewthad]

I agree that turning the empty stream into a stream containing the empty stream is non-intuitive here. But, I really like that the new definition clearly separates the effectful inner stream from the non-effectful outer one.

[andrewthad]

I have the feeling there might also be ways to get evenly divided streams to end up with empty end bits.

I suspect otherwise. Part of the promise of the streaming api is that return () should be indistinguishable from lift (return ()). I think that a function with the behavior you describe would distinguish these two.

[andrewthad]

I think that, despite small differences in behavior, this is better than the current implementation of chunksOf and should replace it.

[treeowl]

Ah yes, I think you're right. I think we probably should make this change, but we should document it a little more carefully.

…

On Thu, Apr 19, 2018 at 11:44 AM, Andrew Martin ***@***.***> wrote: I have the feeling there might also be ways to get evenly divided streams to end up with empty end bits. I suspect otherwise. Part of the promise of the streaming api is that return () should be indistinguishable from lift (return ()). I think that a function with the behavior you describe would distinguish these two. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#46 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABzi_elBfdzoL56baKqYgWy4s8TSD_n0ks5tqLDngaJpZM4TXfwa> .

[torgeirsh]

This is indeed an unfortunate side effect. However, I think it can be solved by using the internal API instead of the external one. This is a quick proof of concept hack:

chunksOf'' :: (Functor f, Monad m, Monad m0) => Int -> Stream f m r -> Stream (Stream f m) m0 r
chunksOf'' n = go where
	go s = case s of
		S.Return r -> pure r
		_ -> do
			stage <- S.yields $ lift . S.inspect =<< S.splitsAt n s
			case stage of
				Left r -> pure r
				Right s' -> go $ S.wrap s'

Does it make sense? I'll try to rewrite the rest in terms of the internal API.

[treeowl]

@torgeirsh, no, I do not think that makes sense. As @andrewthad mentioned, we need to be careful to treat lift (return r) the same as return r.

[torgeirsh]

I see, when pattern matching Effect, a Step is required in order to bind it of course. Seems awkward at first, but I agree there's no other way.

[treeowl]

I don't think chunksOf is the only place we use effects in the outer monad to accomplish some purpose. I haven't quite worked it out yet, but I think we probably do something similar in groups, where we avoid starting off with an empty group.

…

On Thu, Apr 19, 2018 at 3:18 PM, Torgeir Strand Henriksen < ***@***.***> wrote: This is indeed an unfortunate side effect. However, I think it can be solved by using the internal API instead of the external one. This is a quick proof of concept hack: chunksOf'' :: (Functor f, Monad m, Monad m0) => Int -> Stream f m r -> Stream (Stream f m) m0 r chunksOf'' n = go where go s = case s of S.Return r -> pure r _ -> do stage <- S.yields $ lift . S.inspect =<< S.splitsAt n s case stage of Left r -> pure r Right s' -> go $ S.wrap s' Does it make sense? I'll try to rewrite the rest in terms of the internal API. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#46 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABzi_Vrmd5sAeRYK-DgZzcqEKh0d-Pmqks5tqOMBgaJpZM4TXfwa> .

[torgeirsh]

Hmm, I wonder if there should be some kind of policy for this? It might be confusing to have a mix of functions with effectful and pure outer streams.

[treeowl]

Yes, there certainly should be. I'm wondering whether @Gabriel439, @snoyberg, or @ekmett have any thoughts.