Open natalie-o-perret opened 2 years ago
@natalie-o-perret Looks good, please consider making a PR, thanks
You may want to take a close look at what happens if multiple threads are iterating with the resulting sequence. I think the code can cause race conditions. There are functions in this library that specifically warn about that, so maybe it’s enough to just say so in the doc comments.
I’m currently considering adding the same to TaskSeq
, then came across this.
You may want to take a close look at what happens if multiple threads are iterating with the resulting sequence. I think the code can cause race conditions. There are functions in this library that specifically warn about that, so maybe it’s enough to just say so in the doc comments.
I’m currently considering adding the same to
TaskSeq
, then came across this.
This is a good point 🤔.
Tbs, it's also the case with the seq
then, innit?
What about the other functions of the AsyncSeq
module?
Are they all safe regarding the race conditions when multiple threads are iterating the resulting sequence?
@natalie-o-perret I’m not sure about seq
, I’d have to check. But I think the premise there is that you get a new enumerator, but that you shouldn’t share this enumerator between threads.
My concern with AsyncSeq is that there’s a much larger chance it’s used in multi threaded scenarios. Not entirely sure what the base premise is with sharing enumerators. While that’s generally code smell, I think this lib ought to be safe in that regard, but I’d have to check.
@dsyme is there a general idea/consensus in the dotnet ecosystem that sequences should be thread safe, but enumerators of these sequences are not (or: aren’t guaranteed to be)?
It waa my understanding that most collections are safe for reading, but not safe for writing. The thing is, in these functions like chunkBy
or cache
, we’re doing both, but the user probably considers it a read operation.
Description
Bumped into this at work the other day, and thought this could be a nice addition to the library (unless it's already part of it under another name which wouldn't be too surprising since I'm pretty good at missing out the very obvious).
Basically the idea would be to add a
chunkBy
function to theAsyncSeq
module that allows to group and yield adjacent items sharing the same key in the gist of what has been done forSeq.chunkBy
in the F#+ library here, i.e.:A naive impl., ahem translation of above I ended up with:
I've checked a bunch of
Seq
more functional approaches which seem to require more allocations: https://stackoverflow.com/a/38495042/4636721And here is a working example:
Sample output:
(Also, I know [random] vertical alignment is dumb but I can't help it)
Wdyt? Do you think this feature is worthy-enough🔨⚡ (or just relevant 🤔) to be part of the library?