Skip to content

Add TaskSeq.concat overloads for seq, list, array, resizearray #237

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 16, 2024
Merged

Add TaskSeq.concat overloads for seq, list, array, resizearray #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
205 changes: 191 additions & 14 deletions src/FSharp.Control.TaskSeq.Test/TaskSeq.Concat.Tests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module TaskSeq.Tests.Concat

open System
open System.Collections.Generic

open Xunit
Expand All @@ -8,7 +9,11 @@ open FsUnit.Xunit
open FSharp.Control

//
// TaskSeq.concat
// TaskSeq.concat - of task seqs
// TaskSeq.concat - of seqs
// TaskSeq.concat - of lists
// TaskSeq.concat - of arrays
// TaskSeq.concat - of resizable arrays
//

let validateSequence ts =
Expand All @@ -20,31 +25,126 @@ let validateSequence ts =

module EmptySeq =
[<Fact>]
let ``Null source is invalid`` () = assertNullArg <| fun () -> TaskSeq.concat null
let ``Null source is invalid (taskseq)`` () =
assertNullArg
<| fun () -> TaskSeq.concat (null: TaskSeq<TaskSeq<_>>)

[<Fact>]
let ``Null source is invalid (seq)`` () =
assertNullArg
<| fun () -> TaskSeq.concat (null: TaskSeq<seq<_>>)

[<Fact>]
let ``Null source is invalid (array)`` () =
assertNullArg
<| fun () -> TaskSeq.concat (null: TaskSeq<array<_>>)

[<Fact>]
let ``Null source is invalid (list)`` () =
assertNullArg
<| fun () -> TaskSeq.concat (null: TaskSeq<list<_>>)

[<Fact>]
let ``Null source is invalid (resizarray)`` () =
assertNullArg
<| fun () -> TaskSeq.concat (null: TaskSeq<ResizeArray<_>>)

[<Theory; ClassData(typeof<TestEmptyVariants>)>]
let ``TaskSeq-concat with empty sequences`` variant =
let ``TaskSeq-concat with nested empty task sequences`` variant =
taskSeq {
yield Gen.getEmptyVariant variant // not yield-bang!
yield Gen.getEmptyVariant variant
yield Gen.getEmptyVariant variant
yield Gen.getEmptyVariant variant
}
|> TaskSeq.concat
|> verifyEmpty

[<Fact>]
let ``TaskSeq-concat with nested empty sequences`` () =
taskSeq {
yield Seq.empty<string>
yield Seq.empty<string>
yield Seq.empty<string>
}
|> TaskSeq.concat
|> verifyEmpty

[<Fact>]
let ``TaskSeq-concat with nested empty arrays`` () =
taskSeq {
yield Array.empty<int>
yield Array.empty<int>
yield Array.empty<int>
}
|> TaskSeq.concat
|> verifyEmpty

[<Fact>]
let ``TaskSeq-concat with nested empty lists`` () =
taskSeq {
yield List.empty<Guid>
yield List.empty<Guid>
yield List.empty<Guid>
}
|> TaskSeq.concat
|> verifyEmpty

[<Fact>]
let ``TaskSeq-concat with multiple nested empty resizable arrays`` () =
taskSeq {
yield ResizeArray(List.empty<byte>)
yield ResizeArray(List.empty<byte>)
yield ResizeArray(List.empty<byte>)
}
|> TaskSeq.concat
|> verifyEmpty

[<Theory; ClassData(typeof<TestEmptyVariants>)>]
let ``TaskSeq-concat with empty source (taskseq)`` variant =
Gen.getEmptyVariant variant
|> TaskSeq.box
|> TaskSeq.cast<IAsyncEnumerable<int>> // task seq is empty so this should not raise
|> TaskSeq.concat
|> verifyEmpty

[<Theory; ClassData(typeof<TestEmptyVariants>)>]
let ``TaskSeq-concat with empty source (seq)`` variant =
Gen.getEmptyVariant variant
|> TaskSeq.box
|> TaskSeq.cast<int seq> // task seq is empty so this should not raise
|> TaskSeq.concat
|> verifyEmpty

[<Theory; ClassData(typeof<TestEmptyVariants>)>]
let ``TaskSeq-concat with top sequence empty`` variant =
let ``TaskSeq-concat with empty source (list)`` variant =
Gen.getEmptyVariant variant
|> TaskSeq.box
|> TaskSeq.cast<IAsyncEnumerable<int>> // casting an int to an enumerable, LOL!
|> TaskSeq.cast<int list> // task seq is empty so this should not raise
|> TaskSeq.concat
|> verifyEmpty

[<Theory; ClassData(typeof<TestEmptyVariants>)>]
let ``TaskSeq-concat with empty source (array)`` variant =
Gen.getEmptyVariant variant
|> TaskSeq.box
|> TaskSeq.cast<int[]> // task seq is empty so this should not raise
|> TaskSeq.concat
|> verifyEmpty

[<Theory; ClassData(typeof<TestEmptyVariants>)>]
let ``TaskSeq-concat with empty source (resizearray)`` variant =
Gen.getEmptyVariant variant
|> TaskSeq.box
|> TaskSeq.cast<ResizeArray<int>> // task seq is empty so this should not raise
|> TaskSeq.concat
|> verifyEmpty


module Immutable =
[<Theory; ClassData(typeof<TestImmTaskSeq>)>]
let ``TaskSeq-concat with three sequences of sequences`` variant =
taskSeq {
yield Gen.getSeqImmutable variant // not yield-bang!
yield Gen.getSeqImmutable variant
yield Gen.getSeqImmutable variant
yield Gen.getSeqImmutable variant
}
Expand All @@ -55,7 +155,7 @@ module Immutable =
let ``TaskSeq-concat with three sequences of sequences and few empties`` variant =
taskSeq {
yield TaskSeq.empty
yield Gen.getSeqImmutable variant // not yield-bang!
yield Gen.getSeqImmutable variant
yield TaskSeq.empty
yield TaskSeq.empty
yield Gen.getSeqImmutable variant
Expand All @@ -69,23 +169,100 @@ module Immutable =
|> TaskSeq.concat
|> validateSequence

module SideEffect =
[<Theory; ClassData(typeof<TestImmTaskSeq>)>]
let ``TaskSeq-concat consumes until the end, including side-effects`` variant =
let ``TaskSeq-concat throws when one of inner task sequence is null`` variant =
fun () ->
taskSeq {
yield Gen.getSeqImmutable variant
yield TaskSeq.empty
yield null
}
|> TaskSeq.concat
|> consumeTaskSeq
|> should throwAsyncExact typeof<NullReferenceException>

module SideEffect =
[<Fact>]
let ``TaskSeq-concat executes side effects of nested (taskseq)`` () =
let mutable i = 0

taskSeq {
yield Gen.getSeqImmutable variant // not yield-bang!
yield Gen.getSeqImmutable variant
yield Gen.getSeqImmutable SeqImmutable.ThreadSpinWait
yield Gen.getSeqImmutable SeqImmutable.ThreadSpinWait

yield taskSeq {
yield! [ 1..10 ]
i <- i + 1
}
}
|> TaskSeq.concat
|> validateSequence
|> Task.map (fun () -> i |> should equal 1)
|> TaskSeq.last // consume
|> Task.map (fun _ -> i |> should equal 1)

[<Fact>]
let ``TaskSeq-concat executes side effects of nested (seq)`` () =
let mutable i = 0

taskSeq {
yield seq { 1..10 }
yield seq { 1..10 }

yield seq {
yield! [ 1..10 ]
i <- i + 1
}
}
|> TaskSeq.concat
|> TaskSeq.last // consume
|> Task.map (fun _ -> i |> should equal 1)

[<Fact>]
let ``TaskSeq-concat executes side effects of nested (array)`` () =
let mutable i = 0

taskSeq {
yield [| 1..10 |]
yield [| 1..10 |]

yield [| yield! [ 1..10 ]; i <- i + 1 |]
}
|> TaskSeq.concat
|> TaskSeq.last // consume
|> Task.map (fun _ -> i |> should equal 1)

[<Fact>]
let ``TaskSeq-concat executes side effects of nested (list)`` () =
let mutable i = 0

taskSeq {
yield [ 1..10 ]
yield [ 1..10 ]

yield [ yield! [ 1..10 ]; i <- i + 1 ]
}
|> TaskSeq.concat
|> TaskSeq.last // consume
|> Task.map (fun _ -> i |> should equal 1)

[<Fact>]
let ``TaskSeq-concat executes side effects of nested (resizearray)`` () =
let mutable i = 0

taskSeq {
yield ResizeArray { 1..10 }
yield ResizeArray { 1..10 }

yield
ResizeArray(
seq {
yield! [ 1..10 ]
i <- i + 1
}
)
}
|> TaskSeq.concat
|> TaskSeq.last // consume
|> Task.map (fun _ -> i |> should equal 1)

[<Theory; ClassData(typeof<TestImmTaskSeq>)>]
let ``TaskSeq-concat consumes side effects in empty sequences`` variant =
Expand Down
36 changes: 36 additions & 0 deletions src/FSharp.Control.TaskSeq/TaskSeq.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,42 @@ type TaskSeq private () =
yield! (ts :> TaskSeq<'T>)
}

static member concat(sources: TaskSeq<'T seq>) = // NOTE: we cannot use flex types on two overloads
Internal.checkNonNull (nameof sources) sources

taskSeq {
for ts in sources do
// no null-check of inner seqs, similar to seq
yield! ts
}

static member concat(sources: TaskSeq<'T[]>) =
Internal.checkNonNull (nameof sources) sources

taskSeq {
for ts in sources do
// no null-check of inner arrays, similar to seq
yield! ts
}

static member concat(sources: TaskSeq<'T list>) =
Internal.checkNonNull (nameof sources) sources

taskSeq {
for ts in sources do
// no null-check of inner lists, similar to seq
yield! ts
}

static member concat(sources: TaskSeq<ResizeArray<'T>>) =
Internal.checkNonNull (nameof sources) sources

taskSeq {
for ts in sources do
// no null-check of inner resize arrays, similar to seq
yield! ts
}

static member append (source1: TaskSeq<'T>) (source2: TaskSeq<'T>) =
Internal.checkNonNull (nameof source1) source1
Internal.checkNonNull (nameof source2) source2
Expand Down
45 changes: 43 additions & 2 deletions src/FSharp.Control.TaskSeq/TaskSeq.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,55 @@ type TaskSeq =

/// <summary>
/// Combines the given task sequence of task sequences and concatenates them end-to-end, to form a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the given task sequence of task sequences" feels like a mouthful, and Combines does not really help (Flattens would be a great one word TL;DR intro to my mind if I didnt know what concat did)
If I was writing something random from scratch:

Yields a task sequence that unrolls each task sequence emanating from the source, concatenating them end to end,

/// new flattened, single task sequence. Each task sequence is awaited item by item, before the next is iterated.
/// new flattened, single task sequence, like <paramref name="TaskSeq.collect id"/>. Each task sequence is
/// awaited and consumed in full, before the next one is iterated.
/// </summary>
///
/// <param name="sources">The input task-sequence-of-task-sequences.</param>
/// <returns>The resulting task sequence.</returns>
/// <returns>The resulting, flattened task sequence.</returns>
/// <exception cref="T:ArgumentNullException">Thrown when the input task sequence of task sequences is null.</exception>
static member concat: sources: TaskSeq<#TaskSeq<'T>> -> TaskSeq<'T>

/// <summary>
/// Combines the given task sequence of sequences and concatenates them end-to-end, to form a
/// new flattened, single task sequence.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also these are Combinators, so I guess they do Combine in that technical sense (which we all know is the best sense!)

Everything is always "new" for all TaskSeq operators (hence my campaign for a keyword like yield conveying the fact that stuff comes in, has treatments applied, and flows out, remaining lazy)
flattened is great

I'm not sure 'new' is a useful word; if there is a 'Combines' in the description, then there is something new happening.
But we are not actually making a thing that is 'new', it's ephemeral

Hence mq quest for a workd like 'renders' or 'yields' to convey that while we are combining things, there is not really anything 'new' except in the sense that any function that does not return its input results in a 'new' value.

Again, just musing...

/// </summary>
///
/// <param name="sources">The input task sequence of sequences.</param>
/// <returns>The resulting, flattened task sequence.</returns>
/// <exception cref="T:ArgumentNullException">Thrown when the input task sequence of task sequences is null.</exception>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess Seq doesnt either but I'd like to see mention of NRE happening if there is a null in the input
esp for arrays.
But then LINQ SelectMany also does not document it and behaves the same way
So I guess that's case closed based on prior art
(I can recall writing test snippets in 2007 because of this not being written anywhere I was aware of)

But kudos for having the test in the first instance so it's all good I guess

static member concat: sources: TaskSeq<'T seq> -> TaskSeq<'T>

/// <summary>
/// Combines the given task sequence of arrays and concatenates them end-to-end, to form a
/// new flattened, single task sequence.
/// </summary>
///
/// <param name="sources">The input task sequence of arrays.</param>
/// <returns>The resulting, flattened task sequence.</returns>
/// <exception cref="T:ArgumentNullException">Thrown when the input task sequence of task sequences is null.</exception>
static member concat: sources: TaskSeq<'T[]> -> TaskSeq<'T>

/// <summary>
/// Combines the given task sequence of lists and concatenates them end-to-end, to form a
/// new flattened, single task sequence.
/// </summary>
///
/// <param name="sources">The input task sequence of lists.</param>
/// <returns>The resulting, flattened task sequence.</returns>
/// <exception cref="T:ArgumentNullException">Thrown when the input task sequence of task sequences is null.</exception>
static member concat: sources: TaskSeq<'T list> -> TaskSeq<'T>

/// <summary>
/// Combines the given task sequence of resizable arrays and concatenates them end-to-end, to form a
/// new flattened, single task sequence.
/// </summary>
///
/// <param name="sources">The input task sequence of resizable arrays.</param>
/// <returns>The resulting, flattened task sequence.</returns>
/// <exception cref="T:ArgumentNullException">Thrown when the input task sequence of task sequences is null.</exception>
static member concat: sources: TaskSeq<ResizeArray<'T>> -> TaskSeq<'T>

/// <summary>
/// Concatenates task sequences <paramref name="source1" /> and <paramref name="source2" /> in order as a single
/// task sequence.
Expand Down