Skip to content
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

Implement max|min, maxBy|minBy and maxByAsync|minByAsync #221

Merged
merged 4 commits into from
Dec 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 11 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,10 @@ This is what has been implemented so far, is planned or skipped:
| 🚫 | `mapFoldBack` | | | [note #2](#note2 "Because of the async nature of TaskSeq sequences, iterating from the back would be bad practice. Instead, materialize the sequence to a list or array and then apply the 'Back' iterators.") |
| ✅ [#2][] | `mapi` | `mapi` | `mapiAsync` | |
| | `mapi2` | `mapi2` | `mapi2Async` | |
| | `max` | `max` | | |
| | `maxBy` | `maxBy` | `maxByAsync` | |
| | `min` | `min` | | |
| | `minBy` | `minBy` | `minByAsync` | |
| ✅ [#221][]| `max` | `max` | | |
| ✅ [#221][]| `maxBy` | `maxBy` | `maxByAsync` | |
| ✅ [#221][]| `min` | `min` | | |
| ✅ [#221][]| `minBy` | `minBy` | `minByAsync` | |
| ✅ [#2][] | `ofArray` | `ofArray` | | |
| ✅ [#2][] | | `ofAsyncArray` | | |
| ✅ [#2][] | | `ofAsyncList` | | |
Expand Down Expand Up @@ -511,7 +511,12 @@ module TaskSeq =
val mapAsync: mapper: ('T -> #Task<'U>) -> source: TaskSeq<'T> -> TaskSeq<'U>
val mapi: mapper: (int -> 'T -> 'U) -> source: TaskSeq<'T> -> TaskSeq<'U>
val mapiAsync: mapper: (int -> 'T -> #Task<'U>) -> source: TaskSeq<'T> -> TaskSeq<'U>
val ofArray: source: 'T[] -> TaskSeq<'T>
val max: source: TaskSeq<'T> -> Task<'T> when 'T: comparison
val max: source: TaskSeq<'T> -> Task<'T> when 'T: comparison
val maxBy: projection: ('T -> 'U) -> source: TaskSeq<'T> -> Task<'T> when 'U: comparison
val minBy: projection: ('T -> 'U) -> source: TaskSeq<'T> -> Task<'T> when 'U: comparison
val maxByAsync: projection: ('T -> #Task<'U>) -> source: TaskSeq<'T> -> Task<'T> when 'U: comparison
val minByAsync: projection: ('T -> #Task<'U>) -> source: TaskSeq<'T> -> Task<'T> when 'U: comparison val ofArray: source: 'T[] -> TaskSeq<'T>
val ofAsyncArray: source: Async<'T> array -> TaskSeq<'T>
val ofAsyncList: source: Async<'T> list -> TaskSeq<'T>
val ofAsyncSeq: source: seq<Async<'T>> -> TaskSeq<'T>
Expand Down Expand Up @@ -604,6 +609,7 @@ module TaskSeq =
[#209]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/209
[#217]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/217
[#219]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/219
[#221]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/221

[issues]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues
[nuget]: https://www.nuget.org/packages/FSharp.Control.TaskSeq/
9 changes: 5 additions & 4 deletions assets/nuget-package-readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,10 @@ This is what has been implemented so far, is planned or skipped:
| &#x1f6ab; | `mapFoldBack` | | | [note #2](#note2 "Because of the async nature of TaskSeq sequences, iterating from the back would be bad practice. Instead, materialize the sequence to a list or array and then apply the 'Back' iterators.") |
| &#x2705; [#2][] | `mapi` | `mapi` | `mapiAsync` | |
| | `mapi2` | `mapi2` | `mapi2Async` | |
| | `max` | `max` | | |
| | `maxBy` | `maxBy` | `maxByAsync` | |
| | `min` | `min` | | |
| | `minBy` | `minBy` | `minByAsync` | |
| &#x2705; [#221][]| `max` | `max` | | |
| &#x2705; [#221][]| `maxBy` | `maxBy` | `maxByAsync` | |
| &#x2705; [#221][]| `min` | `min` | | |
| &#x2705; [#221][]| `minBy` | `minBy` | `minByAsync` | |
| &#x2705; [#2][] | `ofArray` | `ofArray` | | |
| &#x2705; [#2][] | | `ofAsyncArray` | | |
| &#x2705; [#2][] | | `ofAsyncList` | | |
Expand Down Expand Up @@ -309,3 +309,4 @@ _The motivation for `readOnly` in `Seq` is that a cast from a mutable array or l
[#209]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/209
[#217]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/217
[#219]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/219
[#221]: https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/221
1 change: 1 addition & 0 deletions release-notes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Release notes:
* TaskSeq.truncate, drop, #209
* TaskSeq.where, whereAsync, #217
* TaskSeq.skipWhile, skipWhileInclusive, skipWhileAsync, skipWhileInclusiveAsync, #219
* TaskSeq.max, min, maxBy, minBy, maxByAsync, minByAsync, #221

- Performance: less thread hops with 'StartImmediateAsTask' instead of 'StartAsTask', fixes #135
- BINARY INCOMPATIBILITY: 'TaskSeq' module is now static members on 'TaskSeq<_>', fixes #184
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
<Compile Include="TaskSeq.Last.Tests.fs" />
<Compile Include="TaskSeq.Length.Tests.fs" />
<Compile Include="TaskSeq.Map.Tests.fs" />
<Compile Include="TaskSeq.MaxMin.Tests.fs" />
<Compile Include="TaskSeq.OfXXX.Tests.fs" />
<Compile Include="TaskSeq.Pick.Tests.fs" />
<Compile Include="TaskSeq.Singleton.Tests.fs" />
Expand Down
317 changes: 317 additions & 0 deletions src/FSharp.Control.TaskSeq.Test/TaskSeq.MaxMin.Tests.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,317 @@
module TaskSeq.Tests.MaxMin

open System

open Xunit
open FsUnit.Xunit

open FSharp.Control

//
// TaskSeq.max
// TaskSeq.min
// TaskSeq.maxBy
// TaskSeq.minBy
// TaskSeq.maxByAsync
// TaskSeq.minByAsync
//

type MinMax =
| Max = 0
| Min = 1
| MaxBy = 2
| MinBy = 3
| MaxByAsync = 4
| MinByAsync = 5

module MinMax =
let getFunction =
function
| MinMax.Max -> TaskSeq.max
| MinMax.Min -> TaskSeq.min
| MinMax.MaxBy -> TaskSeq.maxBy id
| MinMax.MinBy -> TaskSeq.minBy id
| MinMax.MaxByAsync -> TaskSeq.maxByAsync Task.fromResult
| MinMax.MinByAsync -> TaskSeq.minByAsync Task.fromResult
| _ -> failwith "impossible"

let getByFunction =
function
| MinMax.MaxBy -> TaskSeq.maxBy
| MinMax.MinBy -> TaskSeq.minBy
| MinMax.MaxByAsync -> fun by -> TaskSeq.maxByAsync (by >> Task.fromResult)
| MinMax.MinByAsync -> fun by -> TaskSeq.minByAsync (by >> Task.fromResult)
| _ -> failwith "impossible"

let getAll () =
[ MinMax.Max; MinMax.Min; MinMax.MaxBy; MinMax.MinBy; MinMax.MaxByAsync; MinMax.MinByAsync ]
|> List.map getFunction

let getAllMin () =
[ MinMax.Min; MinMax.MinBy; MinMax.MinByAsync ]
|> List.map getFunction

let getAllMax () =
[ MinMax.Max; MinMax.MaxBy; MinMax.MaxByAsync ]
|> List.map getFunction

let isMin =
function
| MinMax.Min
| MinMax.MinBy
| MinMax.MinByAsync -> true
| _ -> false

let isMax = isMin >> not


type AllMinMaxFunctions() as this =
inherit TheoryData<MinMax>()

do
this.Add MinMax.Max
this.Add MinMax.Min
this.Add MinMax.MaxBy
this.Add MinMax.MinBy
this.Add MinMax.MaxByAsync
this.Add MinMax.MinByAsync

type JustMin() as this =
inherit TheoryData<MinMax>()

do
this.Add MinMax.Min
this.Add MinMax.MinBy
this.Add MinMax.MinByAsync

type JustMax() as this =
inherit TheoryData<MinMax>()

do
this.Add MinMax.Max
this.Add MinMax.MaxBy
this.Add MinMax.MaxByAsync

type JustMinMaxBy() as this =
inherit TheoryData<MinMax>()

do
this.Add MinMax.MaxBy
this.Add MinMax.MinBy
this.Add MinMax.MaxByAsync
this.Add MinMax.MinByAsync

module EmptySeq =
[<Theory; ClassData(typeof<AllMinMaxFunctions>)>]
let ``Null source raises ArgumentNullException`` (minMaxType: MinMax) =
let minMax = MinMax.getFunction minMaxType

assertNullArg <| fun () -> minMax (null: TaskSeq<int>)

[<Theory; ClassData(typeof<TestEmptyVariants>)>]
let ``Empty sequence raises ArgumentException`` variant =
let test minMax =
let data = Gen.getEmptyVariant variant

fun () -> minMax data |> Task.ignore
|> should throwAsyncExact typeof<ArgumentException>

for minMax in MinMax.getAll () do
test minMax

module Functionality =
[<Fact>]
let ``TaskSeq-max should return maximum`` () = task {
let ts = [ 'A' .. 'Z' ] |> TaskSeq.ofList
let! max = TaskSeq.max ts
max |> should equal 'Z'
}

[<Fact>]
let ``TaskSeq-maxBy should return maximum of input, not the projection`` () = task {
let ts = [ 'A' .. 'Z' ] |> TaskSeq.ofList
let! max = TaskSeq.maxBy id ts
max |> should equal 'Z'

let ts = [ 1..10 ] |> TaskSeq.ofList
let! max = TaskSeq.maxBy (~-) ts
max |> should equal 1 // as negated, -1 is highest, should not return projection, but original
}

[<Fact>]
let ``TaskSeq-maxByAsync should return maximum of input, not the projection`` () = task {
let ts = [ 'A' .. 'Z' ] |> TaskSeq.ofList
let! max = TaskSeq.maxByAsync Task.fromResult ts
max |> should equal 'Z'

let ts = [ 1..10 ] |> TaskSeq.ofList
let! max = TaskSeq.maxByAsync (fun x -> Task.fromResult -x) ts
max |> should equal 1 // as negated, -1 is highest, should not return projection, but original
}

[<Fact>]
let ``TaskSeq-min should return minimum`` () = task {
let ts = [ 'A' .. 'Z' ] |> TaskSeq.ofList
let! min = TaskSeq.min ts
min |> should equal 'A'
}

[<Fact>]
let ``TaskSeq-minBy should return minimum of input, not the projection`` () = task {
let ts = [ 'A' .. 'Z' ] |> TaskSeq.ofList
let! min = TaskSeq.minBy id ts
min |> should equal 'A'

let ts = [ 1..10 ] |> TaskSeq.ofList
let! min = TaskSeq.minBy (~-) ts
min |> should equal 10 // as negated, -10 is lowest, should not return projection, but original
}

[<Fact>]
let ``TaskSeq-minByAsync should return minimum of input, not the projection`` () = task {
let ts = [ 'A' .. 'Z' ] |> TaskSeq.ofList
let! min = TaskSeq.minByAsync Task.fromResult ts
min |> should equal 'A'

let ts = [ 1..10 ] |> TaskSeq.ofList
let! min = TaskSeq.minByAsync (fun x -> Task.fromResult -x) ts
min |> should equal 10 // as negated, 1 is highest, should not return projection, but original
}


module Immutable =
[<Theory; ClassData(typeof<TestImmTaskSeq>)>]
let ``TaskSeq-max, maxBy, maxByAsync returns maximum`` variant = task {
let ts = Gen.getSeqImmutable variant

for max in MinMax.getAllMax () do
let! max = max ts
max |> should equal 10
}

[<Theory; ClassData(typeof<TestImmTaskSeq>)>]
let ``TaskSeq-min, minBy, minByAsync returns minimum`` variant = task {
let ts = Gen.getSeqImmutable variant

for min in MinMax.getAllMin () do
let! min = min ts
min |> should equal 1
}

[<Theory; ClassData(typeof<TestImmTaskSeq>)>]
let ``TaskSeq-maxBy, maxByAsync returns maximum after projection`` variant = task {
let ts = Gen.getSeqImmutable variant
let! max = ts |> TaskSeq.maxBy (fun x -> -x)
max |> should equal 1 // because -1 maps to item '1'
}


[<Theory; ClassData(typeof<TestImmTaskSeq>)>]
let ``TaskSeq-minBy, minByAsync returns minimum after projection`` variant = task {
let ts = Gen.getSeqImmutable variant
let! min = ts |> TaskSeq.minBy (fun x -> -x)
min |> should equal 10 // because -10 maps to item 10
}

module SideSeffects =
[<Fact>]
let ``TaskSeq-max, maxBy, maxByAsync prove we execute after-effects`` () = task {
let mutable i = 0

let ts = taskSeq {
i <- i + 1
i <- i + 1
yield i // 2
i <- i + 1
yield i // 3
yield i + 1 // 4
i <- i + 1 // we should get here
}

do! ts |> TaskSeq.max |> Task.map (should equal 4)
do! ts |> TaskSeq.maxBy (~-) |> Task.map (should equal 6) // next iteration & negation "-6" maps to "6"

do!
ts
|> TaskSeq.maxByAsync Task.fromResult
|> Task.map (should equal 12) // no negation

i |> should equal 12
}

[<Fact>]
let ``TaskSeq-min, minBy, minByAsync prove we execute after-effects test`` () = task {
let mutable i = 0

let ts = taskSeq {
i <- i + 1
i <- i + 1
yield i // 2
i <- i + 1
yield i // 3
yield i + 1 // 4
i <- i + 1 // we should get here
}

do! ts |> TaskSeq.min |> Task.map (should equal 2)
do! ts |> TaskSeq.minBy (~-) |> Task.map (should equal 8) // next iteration & negation

do!
ts
|> TaskSeq.minByAsync Task.fromResult
|> Task.map (should equal 10) // no negation

i |> should equal 12
}


[<Theory; ClassData(typeof<JustMax>)>]
let ``TaskSeq-max with sequence that changes length`` (minMax: MinMax) = task {
let max = MinMax.getFunction minMax
let mutable i = 0

let ts = taskSeq {
i <- i + 10
yield! [ 1..i ]
}

do! max ts |> Task.map (should equal 10)
do! max ts |> Task.map (should equal 20) // mutable state dangers!!
do! max ts |> Task.map (should equal 30) // id
}

[<Theory; ClassData(typeof<JustMin>)>]
let ``TaskSeq-min with sequence that changes length`` (minMax: MinMax) = task {
let min = MinMax.getFunction minMax
let mutable i = 0

let ts = taskSeq {
i <- i + 10
yield! [ 1..i ]
}

do! min ts |> Task.map (should equal 1)
do! min ts |> Task.map (should equal 1) // same min after changing state
do! min ts |> Task.map (should equal 1) // id
}

[<Theory; ClassData(typeof<JustMinMaxBy>)>]
let ``TaskSeq-minBy, maxBy with sequence that changes length`` (minMax: MinMax) = task {

Check warning on line 299 in src/FSharp.Control.TaskSeq.Test/TaskSeq.MaxMin.Tests.fs

View workflow job for this annotation

GitHub Actions / Test Debug Build

This state machine is not statically compilable. A constrained generic construct occured in the resumable code specification. An alternative dynamic implementation will be used, which may be slower. Consider adjusting your code to ensure this state machine is statically compilable, or else suppress this warning.
let mutable i = 0

let ts = taskSeq {
i <- i + 10
yield! [ 1..i ]
}

let test minMaxFn v =
if MinMax.isMin minMax then
// this ensures the "min" version behaves like the "max" version
minMaxFn (~-) ts |> Task.map (should equal v)
else
minMaxFn id ts |> Task.map (should equal v)

do! test (MinMax.getByFunction minMax) 10
do! test (MinMax.getByFunction minMax) 20
do! test (MinMax.getByFunction minMax) 30
}
Loading