Sort functions

[channaj]

This PR implements sort, sortBy, sortDescending and sortByDescending functions and addresses the issue/feature request #126

I have assumed that AsyncSeq is more about asynchronous nature of the type than the infinite nature of it. Therefore, I have implemented these using Seq.sort, Seq.sortBy, Seq.sortDescending and Seq.sortByDescending i.e. I have not considered implementing some sort of time-windowed sorting feature. Like their Seq counterparts, these functions are not intended to be used in large or infinite sequences.

In terms of unit testing, I have added just a test each for testing integer sequences. This feels like a good usecase for property-based testing using a framework like FsCheck but I haven't seen any property-based tests here so decided not to bother. Please let me know if you think introducing property-based tests would add value to this repo and I will bring in FsCheck and add some more tests for the above functions.

Thank you 🙂

[dsyme]

I have assumed that AsyncSeq is more about asynchronous nature of the type than the infinite nature of it. Therefore, I have implemented these using Seq.sort, Seq.sortBy, Seq.sortDescending and Seq.sortByDescending

It feels like these should have seq return type. They don't maintain async - the entire sequence is sucked in synchronously by the Seq.sortBy calls etc. So there's nothing async about them?

I have not considered implementing some sort of time-windowed sorting feature. Like their Seq counterparts, these functions are not intended to be used in large or infinite sequences.

Windowed sorting/grouping with incremental diff suitable for infinite collections would be kind of cool but should likely be part of integrating with a library like FSharp.Data.Adaptive where incremental changes between collections can be part of the computational structure?

[channaj]

Thanks so much for the feedback @dsyme. I'll update these to return seq but could you please validate my thinking below as well?

Yes, it probably makes more sense for them to have seq return type in that perspective. I was thinking of these as more of convenience functions that allows one to invoke while staying in AsyncSeq. I think where I'm coming from is how these functions are implemented in Seq module. For an example Seq.sort converts the input sequence to an array before sorting it, which iterates the whole sequence, loosing it's infinite nature but it still returns a seq instead of an array or a list type, making sure that the type stays in the original type which is seq. Am I not understanding something correctly?

[dsyme]

I think where I'm coming from is how these functions are implemented in Seq module. For an example Seq.sort converts the input sequence to an array before sorting it, which iterates the whole sequence, loosing it's infinite nature but it still returns a seq instead of an array or a list type, making sure that the type stays in the original type which is seq. Am I not understanding something correctly?

Yes that's a reasonable consideration - the Seq functions certainly lose the on-demand nature of the computation.

However somehow it feels a little too much to lose both async and on-demand. It feels like the code is always going to be clearer if at least one of those information-losses is made explicit. This is partly because I belive AsyncSeq is in practice used on infinite lists of events (listening to data updates) more than Seq is (relative to overall use).

[channaj]

Thanks for clarifying that. I've updated them to return seq

[channaj]

@dsyme please take a look when you have a minute. Thanks.

[dsyme]

Hmmmm.... since they all use toArrayAsync maybe the return type should just be an honest array!

[channaj]

Yes, done 👍