From 04d05c746c60aade17382057a66e0946c011234f Mon Sep 17 00:00:00 2001 From: Leo Conforti Date: Tue, 27 Oct 2026 16:13:01 -0600 Subject: [PATCH 1/5] fix(js/ts): break min and max ties like FSharp.Core - List, Array and Seq minBy returned the last of the tied minima; they now return the first, and minBy and maxBy call the projection once per element instead of twice per step. - List, Array and Seq min returned the last of equal minima, visible with 1.0m and 1.00m, -0.0 and 0.0 or a custom comparison. They now return the first, like max already did. - The max operator returned its second argument when the values compare equal; it now returns the first like FSharp.Core (min already returned the second, as FSharp.Core does). --- src/fable-library-ts/Array.fs | 52 +++++++++++++++++---------- src/fable-library-ts/List.fs | 56 ++++++++++++++++++----------- src/fable-library-ts/Seq.fs | 60 +++++++++++++++++++++----------- src/fable-library-ts/Util.ts | 2 +- tests/Js/Main/ArrayTests.fs | 21 +++++++++++ tests/Js/Main/ComparisonTests.fs | 27 ++++++++++++++ tests/Js/Main/ListTests.fs | 21 +++++++++++ tests/Js/Main/SeqTests.fs | 21 +++++++++++ 8 files changed, 198 insertions(+), 62 deletions(-) diff --git a/src/fable-library-ts/Array.fs b/src/fable-library-ts/Array.fs index fd7cbe5a3a..2f99b04360 100644 --- a/src/fable-library-ts/Array.fs +++ b/src/fable-library-ts/Array.fs @@ -1056,14 +1056,21 @@ let sumBy (projection: 'T -> 'T2) (array: 'T[]) ([] adder: IGenericAdder acc let maxBy (projection: 'a -> 'b) (xs: 'a[]) ([] comparer: IComparer<'b>) : 'a = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - y - else - x - ) - xs + if xs.Length = 0 then + invalidArg "array" LanguagePrimitives.ErrorStrings.InputArrayEmptyString + + let mutable acc = xs.[0] + let mutable accv = projection acc + + for i = 1 to xs.Length - 1 do + let x = xs.[i] + let v = projection x + + if comparer.Compare(v, accv) > 0 then + acc <- x + accv <- v + + acc let max (xs: 'a[]) ([] comparer: IComparer<'a>) : 'a = reduce @@ -1076,22 +1083,29 @@ let max (xs: 'a[]) ([] comparer: IComparer<'a>) : 'a = xs let minBy (projection: 'a -> 'b) (xs: 'a[]) ([] comparer: IComparer<'b>) : 'a = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - x - else - y - ) - xs + if xs.Length = 0 then + invalidArg "array" LanguagePrimitives.ErrorStrings.InputArrayEmptyString + + let mutable acc = xs.[0] + let mutable accv = projection acc + + for i = 1 to xs.Length - 1 do + let x = xs.[i] + let v = projection x + + if comparer.Compare(v, accv) < 0 then + acc <- x + accv <- v + + acc let min (xs: 'a[]) ([] comparer: IComparer<'a>) : 'a = reduce (fun x y -> - if comparer.Compare(y, x) > 0 then - x - else + if comparer.Compare(y, x) < 0 then y + else + x ) xs diff --git a/src/fable-library-ts/List.fs b/src/fable-library-ts/List.fs index 3cf4d74945..0133bcb37f 100644 --- a/src/fable-library-ts/List.fs +++ b/src/fable-library-ts/List.fs @@ -685,15 +685,22 @@ let sum (xs: 'T list) ([] adder: IGenericAdder<'T>) : 'T = let sumBy (f: 'T -> 'U) (xs: 'T list) ([] adder: IGenericAdder<'U>) : 'U = fold (fun acc x -> adder.Add(acc, f x)) (adder.GetZero()) xs -let maxBy (projection: 'T -> 'U) xs ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - y - else - x - ) - xs +let maxBy (projection: 'T -> 'U) (xs: 'T list) ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = + let mutable acc = head xs + let mutable accv = projection acc + let mutable xs = tail xs + + while not xs.IsEmpty do + let x = head xs + let v = projection x + + if comparer.Compare(v, accv) > 0 then + acc <- x + accv <- v + + xs <- xs.Tail + + acc let max xs ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T = reduce @@ -705,23 +712,30 @@ let max xs ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T ) xs -let minBy (projection: 'T -> 'U) xs ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - x - else - y - ) - xs +let minBy (projection: 'T -> 'U) (xs: 'T list) ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = + let mutable acc = head xs + let mutable accv = projection acc + let mutable xs = tail xs + + while not xs.IsEmpty do + let x = head xs + let v = projection x + + if comparer.Compare(v, accv) < 0 then + acc <- x + accv <- v + + xs <- xs.Tail + + acc let min (xs: 'T list) ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T = reduce (fun x y -> - if comparer.Compare(y, x) > 0 then - x - else + if comparer.Compare(y, x) < 0 then y + else + x ) xs diff --git a/src/fable-library-ts/Seq.fs b/src/fable-library-ts/Seq.fs index cf14a251fd..a3e01f3ca9 100644 --- a/src/fable-library-ts/Seq.fs +++ b/src/fable-library-ts/Seq.fs @@ -1172,15 +1172,24 @@ let sum (xs: seq<'T>) ([] adder: IGenericAdder<'T>) : 'T = let sumBy (f: 'T -> 'U) (xs: seq<'T>) ([] adder: IGenericAdder<'U>) : 'U = fold (fun acc x -> adder.Add(acc, f x)) (adder.GetZero()) xs -let maxBy (projection: 'T -> 'U) xs ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - y - else - x - ) - xs +let maxBy (projection: 'T -> 'U) (xs: seq<'T>) ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = + use e = ofSeq xs + + if not (e.MoveNext()) then + invalidArg "source" SR.inputSequenceEmpty + + let mutable acc = e.Current + let mutable accv = projection acc + + while e.MoveNext() do + let x = e.Current + let v = projection x + + if comparer.Compare(v, accv) > 0 then + acc <- x + accv <- v + + acc let max xs ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T = reduce @@ -1192,23 +1201,32 @@ let max xs ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T ) xs -let minBy (projection: 'T -> 'U) xs ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - x - else - y - ) - xs +let minBy (projection: 'T -> 'U) (xs: seq<'T>) ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = + use e = ofSeq xs + + if not (e.MoveNext()) then + invalidArg "source" SR.inputSequenceEmpty + + let mutable acc = e.Current + let mutable accv = projection acc + + while e.MoveNext() do + let x = e.Current + let v = projection x + + if comparer.Compare(v, accv) < 0 then + acc <- x + accv <- v + + acc let min (xs: seq<'T>) ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T = reduce (fun x y -> - if comparer.Compare(y, x) > 0 then - x - else + if comparer.Compare(y, x) < 0 then y + else + x ) xs diff --git a/src/fable-library-ts/Util.ts b/src/fable-library-ts/Util.ts index 802e86dbbb..97eb591e24 100644 --- a/src/fable-library-ts/Util.ts +++ b/src/fable-library-ts/Util.ts @@ -673,7 +673,7 @@ export function min(comparer: (x: T, y: T) => number, x: T, y: T) { } export function max(comparer: (x: T, y: T) => number, x: T, y: T) { - return comparer(x, y) > 0 ? x : y; + return comparer(x, y) < 0 ? y : x; } export function clamp(comparer: (x: T, y: T) => number, value: T, min: T, max: T) { diff --git a/tests/Js/Main/ArrayTests.fs b/tests/Js/Main/ArrayTests.fs index 9f5fc8cbac..951469d595 100644 --- a/tests/Js/Main/ArrayTests.fs +++ b/tests/Js/Main/ArrayTests.fs @@ -871,6 +871,27 @@ let tests = xs |> Array.minBy (fun x -> -x) |> equal 2. + testCase "Array.minBy and Array.maxBy keep the first of tied elements" <| fun () -> + let xs = [| "a", 1; "b", 0; "c", 0; "d", 1 |] + xs |> Array.minBy snd |> equal ("b", 0) + xs |> Array.maxBy snd |> equal ("a", 1) + xs |> Array.minBy (fun _ -> 0) |> equal ("a", 1) + + testCase "Array.minBy and Array.maxBy call the projection once per element" <| fun () -> + let mutable calls = 0 + let f x = calls <- calls + 1; x + [| 5; 3; 8; 1; 9 |] |> Array.minBy f |> equal 1 + [| 5; 3; 8; 1; 9 |] |> Array.maxBy f |> equal 9 + calls |> equal 10 + + testCase "Array.min and Array.max keep the first of equal elements" <| fun () -> + Array.min [| 1.0m; 1.00m |] |> string |> equal "1.0" + Array.max [| 1.00m; 1.0m |] |> string |> equal "1.00" + // Computed, since the standalone compiler writes a -0. literal as 0 + let negZero = 1. / System.Double.NegativeInfinity + 1. / Array.min [| negZero; 0. |] |> equal System.Double.NegativeInfinity + 1. / Array.max [| 0.; negZero |] |> equal System.Double.PositiveInfinity + testCase "Array.ofList works" <| fun () -> let xs = [1.; 2.] let ys = Array.ofList xs diff --git a/tests/Js/Main/ComparisonTests.fs b/tests/Js/Main/ComparisonTests.fs index 39ee5d9cf3..785a1499d4 100644 --- a/tests/Js/Main/ComparisonTests.fs +++ b/tests/Js/Main/ComparisonTests.fs @@ -101,6 +101,23 @@ type CustomValue(value: int, label: string) = override _.GetHashCode() = value +type RankedValue(rank: int, label: string) = + member _.Rank = rank + member _.Label = label + + override _.Equals(other) = + match other with + | :? RankedValue as other -> rank = other.Rank + | _ -> false + + override _.GetHashCode() = rank + + interface System.IComparable with + member _.CompareTo(other) = + match other with + | :? RankedValue as other -> compare rank other.Rank + | _ -> invalidArg "other" "cannot compare values of different types" + type WrappedUnion = WrappedUnion of CustomValue type WrappedRecord = { Value: CustomValue } @@ -515,6 +532,16 @@ let tests = let c2 = Test(5) Object.ReferenceEquals(min c1 c2, c2) |> equal true + testCase "max and min of equal values return the same argument as .NET" <| fun () -> + let x = RankedValue(0, "x") + let y = RankedValue(0, "y") + (max x y).Label |> equal "x" + (min x y).Label |> equal "y" + (max (0, x) (0, y) |> snd).Label |> equal "x" + (min (0, x) (0, y) |> snd).Label |> equal "y" + max 1.0m 1.00m |> string |> equal "1.0" + min 1.0m 1.00m |> string |> equal "1.00" + testCase "nullArg works" <| fun () -> try nullArg null diff --git a/tests/Js/Main/ListTests.fs b/tests/Js/Main/ListTests.fs index 4df3113faf..da66ed7bae 100644 --- a/tests/Js/Main/ListTests.fs +++ b/tests/Js/Main/ListTests.fs @@ -417,6 +417,27 @@ let tests = xs |> List.minBy (fun x -> -x) |> equal 2 + testCase "List.minBy and List.maxBy keep the first of tied elements" <| fun () -> + let xs = [ "a", 1; "b", 0; "c", 0; "d", 1 ] + xs |> List.minBy snd |> equal ("b", 0) + xs |> List.maxBy snd |> equal ("a", 1) + xs |> List.minBy (fun _ -> 0) |> equal ("a", 1) + + testCase "List.minBy and List.maxBy call the projection once per element" <| fun () -> + let mutable calls = 0 + let f x = calls <- calls + 1; x + [ 5; 3; 8; 1; 9 ] |> List.minBy f |> equal 1 + [ 5; 3; 8; 1; 9 ] |> List.maxBy f |> equal 9 + calls |> equal 10 + + testCase "List.min and List.max keep the first of equal elements" <| fun () -> + List.min [ 1.0m; 1.00m ] |> string |> equal "1.0" + List.max [ 1.00m; 1.0m ] |> string |> equal "1.00" + // Computed, since the standalone compiler writes a -0. literal as 0 + let negZero = 1. / System.Double.NegativeInfinity + 1. / List.min [ negZero; 0. ] |> equal System.Double.NegativeInfinity + 1. / List.max [ 0.; negZero ] |> equal System.Double.PositiveInfinity + testCase "List.sum works" <| fun () -> [1; 2] |> List.sum |> equal 3 diff --git a/tests/Js/Main/SeqTests.fs b/tests/Js/Main/SeqTests.fs index fc19f494ba..fa8951ca5f 100644 --- a/tests/Js/Main/SeqTests.fs +++ b/tests/Js/Main/SeqTests.fs @@ -404,6 +404,27 @@ let tests = xs |> Seq.minBy (fun x -> -x) |> equal 2. + testCase "Seq.minBy and Seq.maxBy keep the first of tied elements" <| fun () -> + let xs = Seq.ofList [ "a", 1; "b", 0; "c", 0; "d", 1 ] + xs |> Seq.minBy snd |> equal ("b", 0) + xs |> Seq.maxBy snd |> equal ("a", 1) + xs |> Seq.minBy (fun _ -> 0) |> equal ("a", 1) + + testCase "Seq.minBy and Seq.maxBy call the projection once per element" <| fun () -> + let mutable calls = 0 + let f x = calls <- calls + 1; x + seq { 5; 3; 8; 1; 9 } |> Seq.minBy f |> equal 1 + seq { 5; 3; 8; 1; 9 } |> Seq.maxBy f |> equal 9 + calls |> equal 10 + + testCase "Seq.min and Seq.max keep the first of equal elements" <| fun () -> + Seq.min (seq { 1.0m; 1.00m }) |> string |> equal "1.0" + Seq.max (seq { 1.00m; 1.0m }) |> string |> equal "1.00" + // Computed, since the standalone compiler writes a -0. literal as 0 + let negZero = 1. / System.Double.NegativeInfinity + 1. / Seq.min (seq { negZero; 0. }) |> equal System.Double.NegativeInfinity + 1. / Seq.max (seq { 0.; negZero }) |> equal System.Double.PositiveInfinity + testCase "Seq.max with non numeric types works" <| fun () -> let p1 = {x=1; y=1} let p2 = {x=2; y=2} From 324b0cb6c9fb052757c9aaea2e9470527006166b Mon Sep 17 00:00:00 2001 From: Leo Conforti Date: Tue, 27 Oct 2026 16:15:57 -0600 Subject: [PATCH 2/5] fix(python): break min and max ties like FSharp.Core - List and Seq minBy returned the last of the tied minima; they now return the first. List, Array and Seq minBy and maxBy call the projection once per element instead of twice per step. - List and Seq min, and Array min and max, returned the last of equal elements (e.g. -0.0 and 0.0); they now return the first. - The max operator returned its second argument when the values compare equal; it now returns the first like FSharp.Core. --- src/fable-library-py/fable_library/List.fs | 56 ++++++++++++-------- src/fable-library-py/fable_library/Seq.fs | 60 ++++++++++++++-------- src/fable-library-py/fable_library/util.py | 2 +- src/fable-library-py/src/array.rs | 55 +++++++++++++------- tests/Python/TestArray.fs | 20 ++++++++ tests/Python/TestComparison.fs | 26 ++++++++++ tests/Python/TestList.fs | 20 ++++++++ tests/Python/TestSeq.fs | 20 ++++++++ 8 files changed, 198 insertions(+), 61 deletions(-) diff --git a/src/fable-library-py/fable_library/List.fs b/src/fable-library-py/fable_library/List.fs index 9859354847..915340d289 100644 --- a/src/fable-library-py/fable_library/List.fs +++ b/src/fable-library-py/fable_library/List.fs @@ -665,15 +665,22 @@ let sum (xs: 'T list) ([] adder: IGenericAdder<'T>) : 'T = let sumBy (f: 'T -> 'U) (xs: 'T list) ([] adder: IGenericAdder<'U>) : 'U = fold (fun acc x -> adder.Add(acc, f x)) (adder.GetZero()) xs -let maxBy (projection: 'T -> 'U) xs ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - y - else - x - ) - xs +let maxBy (projection: 'T -> 'U) (xs: 'T list) ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = + let mutable acc = head xs + let mutable accv = projection acc + let mutable xs = tail xs + + while not xs.IsEmpty do + let x = head xs + let v = projection x + + if comparer.Compare(v, accv) > 0 then + acc <- x + accv <- v + + xs <- xs.Tail + + acc let max xs ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T = reduce @@ -685,23 +692,30 @@ let max xs ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T ) xs -let minBy (projection: 'T -> 'U) xs ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - x - else - y - ) - xs +let minBy (projection: 'T -> 'U) (xs: 'T list) ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = + let mutable acc = head xs + let mutable accv = projection acc + let mutable xs = tail xs + + while not xs.IsEmpty do + let x = head xs + let v = projection x + + if comparer.Compare(v, accv) < 0 then + acc <- x + accv <- v + + xs <- xs.Tail + + acc let min (xs: 'T list) ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T = reduce (fun x y -> - if comparer.Compare(y, x) > 0 then - x - else + if comparer.Compare(y, x) < 0 then y + else + x ) xs diff --git a/src/fable-library-py/fable_library/Seq.fs b/src/fable-library-py/fable_library/Seq.fs index e15b92cd11..aa0df42a29 100644 --- a/src/fable-library-py/fable_library/Seq.fs +++ b/src/fable-library-py/fable_library/Seq.fs @@ -1019,15 +1019,24 @@ let sum (xs: seq<'T>) ([] adder: IGenericAdder<'T>) : 'T = let sumBy (f: 'T -> 'U) (xs: seq<'T>) ([] adder: IGenericAdder<'U>) : 'U = fold (fun acc x -> adder.Add(acc, f x)) (adder.GetZero()) xs -let maxBy (projection: 'T -> 'U) xs ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - y - else - x - ) - xs +let maxBy (projection: 'T -> 'U) (xs: seq<'T>) ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = + use e = ofSeq xs + + if not (e.MoveNext()) then + invalidArg "source" SR.inputSequenceEmpty + + let mutable acc = e.Current + let mutable accv = projection acc + + while e.MoveNext() do + let x = e.Current + let v = projection x + + if comparer.Compare(v, accv) > 0 then + acc <- x + accv <- v + + acc let max xs ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T = reduce @@ -1039,23 +1048,32 @@ let max xs ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T ) xs -let minBy (projection: 'T -> 'U) xs ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - x - else - y - ) - xs +let minBy (projection: 'T -> 'U) (xs: seq<'T>) ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = + use e = ofSeq xs + + if not (e.MoveNext()) then + invalidArg "source" SR.inputSequenceEmpty + + let mutable acc = e.Current + let mutable accv = projection acc + + while e.MoveNext() do + let x = e.Current + let v = projection x + + if comparer.Compare(v, accv) < 0 then + acc <- x + accv <- v + + acc let min (xs: seq<'T>) ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T = reduce (fun x y -> - if comparer.Compare(y, x) > 0 then - x - else + if comparer.Compare(y, x) < 0 then y + else + x ) xs diff --git a/src/fable-library-py/fable_library/util.py b/src/fable-library-py/fable_library/util.py index e4aaf03d4c..1dcf9e7f54 100644 --- a/src/fable-library-py/fable_library/util.py +++ b/src/fable-library-py/fable_library/util.py @@ -337,7 +337,7 @@ def min[T](comparer: Callable[[T, T], int], x: T, y: T) -> T: def max[T](comparer: Callable[[T, T], int], x: T, y: T) -> T: - return x if comparer(x, y) > 0 else y + return y if comparer(x, y) < 0 else x def clamp[T](comparer: Callable[[T, T], int], value: T, min: T, max: T): diff --git a/src/fable-library-py/src/array.rs b/src/fable-library-py/src/array.rs index 8d68d631f3..57d81ba946 100644 --- a/src/fable-library-py/src/array.rs +++ b/src/fable-library-py/src/array.rs @@ -2959,9 +2959,9 @@ impl FSharpArray { } reduce_impl(self, py, |acc, item, py| { let comparison = - comparer.call_method1("Compare", (acc.clone_ref(py), item.clone_ref(py)))?; + comparer.call_method1("Compare", (item.clone_ref(py), acc.clone_ref(py)))?; let is_gt = comparison.extract::()? > 0; - Ok(if is_gt { acc } else { item }) + Ok(if is_gt { item } else { acc }) }) } @@ -2972,9 +2972,9 @@ impl FSharpArray { } reduce_impl(self, py, |acc, item, py| { let comparison = - comparer.call_method1("Compare", (acc.clone_ref(py), item.clone_ref(py)))?; + comparer.call_method1("Compare", (item.clone_ref(py), acc.clone_ref(py)))?; let is_lt = comparison.extract::()? < 0; - Ok(if is_lt { acc } else { item }) + Ok(if is_lt { item } else { acc }) }) } @@ -2985,13 +2985,7 @@ impl FSharpArray { projection: &Bound<'_, PyAny>, comparer: &Bound<'_, PyAny>, ) -> PyResult> { - reduce_impl(self, py, |acc, item, py| { - let acc_key = projection.call1((acc.clone_ref(py),))?; - let item_key = projection.call1((item.clone_ref(py),))?; - let comparison = comparer.call_method1("Compare", (item_key, acc_key))?; - let is_gt = comparison.extract::()? > 0; - Ok(if is_gt { item } else { acc }) - }) + extreme_by_impl(self, py, projection, comparer, |c| c > 0) } /// Returns the element for which the key function returns the smallest value. @@ -3001,13 +2995,7 @@ impl FSharpArray { projection: &Bound<'_, PyAny>, comparer: &Bound<'_, PyAny>, ) -> PyResult> { - reduce_impl(self, py, |acc, item, py| { - let acc_key = projection.call1((acc.clone_ref(py),))?; - let item_key = projection.call1((item.clone_ref(py),))?; - let comparison = comparer.call_method1("Compare", (item_key, acc_key))?; - let is_lt = comparison.extract::()? < 0; - Ok(if is_lt { item } else { acc }) - }) + extreme_by_impl(self, py, projection, comparer, |c| c < 0) } // Choose implementation (map + filter in one pass) @@ -5609,6 +5597,37 @@ where Ok(acc) } +// Internal minBy/maxBy implementation: projects each element once and keeps the first of tied keys +fn extreme_by_impl( + array: &FSharpArray, + py: Python<'_>, + projection: &Bound<'_, PyAny>, + comparer: &Bound<'_, PyAny>, + replace: F, +) -> PyResult> +where + F: Fn(i32) -> bool, +{ + let len = array.storage.len(); + if len == 0 { + return Err(PyErr::new::( + "Cannot reduce an empty array.", + )); + } + let mut acc = array.get_item_at_index(0, py)?; + let mut acc_key = projection.call1((acc.clone_ref(py),))?; + for i in 1..len { + let item = array.get_item_at_index(i as isize, py)?; + let item_key = projection.call1((item.clone_ref(py),))?; + let comparison = comparer.call_method1("Compare", (&item_key, &acc_key))?; + if replace(comparison.extract::()?) { + acc = item; + acc_key = item_key; + } + } + Ok(acc) +} + /// A module for array operations pub fn register_array_module(parent_module: &Bound<'_, PyModule>) -> PyResult<()> { let m = PyModule::new(parent_module.py(), "array")?; diff --git a/tests/Python/TestArray.fs b/tests/Python/TestArray.fs index c1f68a3c12..3d9230ccf2 100644 --- a/tests/Python/TestArray.fs +++ b/tests/Python/TestArray.fs @@ -869,6 +869,26 @@ let ``test Array.minBy works`` () = xs |> Array.minBy (fun x -> -x) |> equal 2. +[] +let ``test Array.minBy and Array.maxBy keep the first of tied elements`` () = + let xs = [| "a", 1; "b", 0; "c", 0; "d", 1 |] + xs |> Array.minBy snd |> equal ("b", 0) + xs |> Array.maxBy snd |> equal ("a", 1) + xs |> Array.minBy (fun _ -> 0) |> equal ("a", 1) + +[] +let ``test Array.minBy and Array.maxBy call the projection once per element`` () = + let mutable calls = 0 + let f x = calls <- calls + 1; x + [| 5; 3; 8; 1; 9 |] |> Array.minBy f |> equal 1 + [| 5; 3; 8; 1; 9 |] |> Array.maxBy f |> equal 9 + calls |> equal 10 + +[] +let ``test Array.min and Array.max keep the first of equal elements`` () = + 1. / Array.min [| -0.; 0. |] |> equal System.Double.NegativeInfinity + 1. / Array.max [| 0.; -0. |] |> equal System.Double.PositiveInfinity + [] let ``test Array.ofList works`` () = let xs = [1.; 2.] diff --git a/tests/Python/TestComparison.fs b/tests/Python/TestComparison.fs index 9e29ebcb7f..d4a46ee15d 100644 --- a/tests/Python/TestComparison.fs +++ b/tests/Python/TestComparison.fs @@ -109,6 +109,23 @@ type CustomValue(value: int, label: string) = override _.GetHashCode() = value +type RankedValue(rank: int, label: string) = + member _.Rank = rank + member _.Label = label + + override _.Equals(other) = + match other with + | :? RankedValue as other -> rank = other.Rank + | _ -> false + + override _.GetHashCode() = rank + + interface System.IComparable with + member _.CompareTo(other) = + match other with + | :? RankedValue as other -> compare rank other.Rank + | _ -> invalidArg "other" "cannot compare values of different types" + type WrappedUnion = WrappedUnion of CustomValue type WrappedRecord = { Value: CustomValue } @@ -615,6 +632,15 @@ let ``test min with objects implementing IComparable works`` () = let c2 = MyTest(5) Object.ReferenceEquals(min c1 c2, c2) |> equal true +[] +let ``test max and min of equal values return the same argument as .NET`` () = + let x = RankedValue(0, "x") + let y = RankedValue(0, "y") + (max x y).Label |> equal "x" + (min x y).Label |> equal "y" + (max (0, x) (0, y) |> snd).Label |> equal "x" + (min (0, x) (0, y) |> snd).Label |> equal "y" + [] let ``test nullArg works`` () = try diff --git a/tests/Python/TestList.fs b/tests/Python/TestList.fs index c7db8d1859..73a623b8c8 100644 --- a/tests/Python/TestList.fs +++ b/tests/Python/TestList.fs @@ -484,6 +484,26 @@ let ``test List.minBy works`` () = xs |> List.minBy (fun x -> -x) |> equal 2 +[] +let ``test List.minBy and List.maxBy keep the first of tied elements`` () = + let xs = [ "a", 1; "b", 0; "c", 0; "d", 1 ] + xs |> List.minBy snd |> equal ("b", 0) + xs |> List.maxBy snd |> equal ("a", 1) + xs |> List.minBy (fun _ -> 0) |> equal ("a", 1) + +[] +let ``test List.minBy and List.maxBy call the projection once per element`` () = + let mutable calls = 0 + let f x = calls <- calls + 1; x + [ 5; 3; 8; 1; 9 ] |> List.minBy f |> equal 1 + [ 5; 3; 8; 1; 9 ] |> List.maxBy f |> equal 9 + calls |> equal 10 + +[] +let ``test List.min and List.max keep the first of equal elements`` () = + 1. / List.min [ -0.; 0. ] |> equal System.Double.NegativeInfinity + 1. / List.max [ 0.; -0. ] |> equal System.Double.PositiveInfinity + [] let ``test List.sum works`` () = [1; 2] |> List.sum diff --git a/tests/Python/TestSeq.fs b/tests/Python/TestSeq.fs index 78314876de..1c5532c385 100644 --- a/tests/Python/TestSeq.fs +++ b/tests/Python/TestSeq.fs @@ -498,6 +498,26 @@ let ``test Seq.minBy works`` () = xs |> Seq.minBy (fun x -> -x) |> equal 2. +[] +let ``test Seq.minBy and Seq.maxBy keep the first of tied elements`` () = + let xs = Seq.ofList [ "a", 1; "b", 0; "c", 0; "d", 1 ] + xs |> Seq.minBy snd |> equal ("b", 0) + xs |> Seq.maxBy snd |> equal ("a", 1) + xs |> Seq.minBy (fun _ -> 0) |> equal ("a", 1) + +[] +let ``test Seq.minBy and Seq.maxBy call the projection once per element`` () = + let mutable calls = 0 + let f x = calls <- calls + 1; x + seq { 5; 3; 8; 1; 9 } |> Seq.minBy f |> equal 1 + seq { 5; 3; 8; 1; 9 } |> Seq.maxBy f |> equal 9 + calls |> equal 10 + +[] +let ``test Seq.min and Seq.max keep the first of equal elements`` () = + 1. / Seq.min (seq { -0.; 0. }) |> equal System.Double.NegativeInfinity + 1. / Seq.max (seq { 0.; -0. }) |> equal System.Double.PositiveInfinity + [] let ``test Seq.max with non numeric types works`` () = let p1 = {x=1; y=1} From 433ad4fee1eb606703365a916cb52ad44cff45d1 Mon Sep 17 00:00:00 2001 From: Leo Conforti Date: Tue, 27 Oct 2026 16:18:36 -0600 Subject: [PATCH 3/5] fix(dart): break min and max ties like FSharp.Core - List, Array and Seq minBy returned the last of the tied minima; they now return the first, and minBy and maxBy call the projection once per element instead of twice per step. - List, Array and Seq min returned the last of equal elements (e.g. with a custom comparison); they now return the first, like max already did. - The max operator returned its second argument when the values compare equal; it now returns the first like FSharp.Core. --- src/fable-library-dart/Array.fs | 52 +++++++++++++++++---------- src/fable-library-dart/List.fs | 56 ++++++++++++++++++----------- src/fable-library-dart/Seq.fs | 60 ++++++++++++++++++++----------- src/fable-library-dart/Util.dart | 2 +- tests/Dart/src/ArrayTests.fs | 13 +++++++ tests/Dart/src/ComparisonTests.fs | 32 +++++++++++++++++ tests/Dart/src/ListTests.fs | 13 +++++++ tests/Dart/src/SeqTests.fs | 13 +++++++ 8 files changed, 179 insertions(+), 62 deletions(-) diff --git a/src/fable-library-dart/Array.fs b/src/fable-library-dart/Array.fs index db4e3746e8..d390d2554c 100644 --- a/src/fable-library-dart/Array.fs +++ b/src/fable-library-dart/Array.fs @@ -958,14 +958,21 @@ let sumBy (projection: 'T -> 'T2) (array: 'T[]) ([] adder: IGenericAdder acc let maxBy (projection: 'a -> 'b) (xs: 'a[]) ([] comparer: IComparer<'b>) : 'a = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - y - else - x - ) - xs + if Array.isEmpty xs then + invalidArg "array" LanguagePrimitives.ErrorStrings.InputArrayEmptyString + + let mutable acc = xs[0] + let mutable accv = projection acc + + for i = 1 to xs.Length - 1 do + let x = xs[i] + let v = projection x + + if comparer.Compare(v, accv) > 0 then + acc <- x + accv <- v + + acc let max (xs: 'a[]) ([] comparer: IComparer<'a>) : 'a = reduce @@ -978,22 +985,29 @@ let max (xs: 'a[]) ([] comparer: IComparer<'a>) : 'a = xs let minBy (projection: 'a -> 'b) (xs: 'a[]) ([] comparer: IComparer<'b>) : 'a = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - x - else - y - ) - xs + if Array.isEmpty xs then + invalidArg "array" LanguagePrimitives.ErrorStrings.InputArrayEmptyString + + let mutable acc = xs[0] + let mutable accv = projection acc + + for i = 1 to xs.Length - 1 do + let x = xs[i] + let v = projection x + + if comparer.Compare(v, accv) < 0 then + acc <- x + accv <- v + + acc let min (xs: 'a[]) ([] comparer: IComparer<'a>) : 'a = reduce (fun x y -> - if comparer.Compare(y, x) > 0 then - x - else + if comparer.Compare(y, x) < 0 then y + else + x ) xs diff --git a/src/fable-library-dart/List.fs b/src/fable-library-dart/List.fs index 50c0948e56..785fa16d7d 100644 --- a/src/fable-library-dart/List.fs +++ b/src/fable-library-dart/List.fs @@ -640,15 +640,22 @@ let sum (xs: 'T list) ([] adder: IGenericAdder<'T>) : 'T = let sumBy (f: 'T -> 'U) (xs: 'T list) ([] adder: IGenericAdder<'U>) : 'U = fold (fun acc x -> adder.Add(acc, f x)) (adder.GetZero()) xs -let maxBy (projection: 'T -> 'U) xs ([] comparer: IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - y - else - x - ) - xs +let maxBy (projection: 'T -> 'U) (xs: 'T list) ([] comparer: IComparer<'U>) : 'T = + let mutable acc = head xs + let mutable accv = projection acc + let mutable xs = tail xs + + while not xs.IsEmpty do + let x = xs.Head + let v = projection x + + if comparer.Compare(v, accv) > 0 then + acc <- x + accv <- v + + xs <- xs.Tail + + acc let max xs ([] comparer: IComparer<'T>) : 'T = reduce @@ -660,23 +667,30 @@ let max xs ([] comparer: IComparer<'T>) : 'T = ) xs -let minBy (projection: 'T -> 'U) xs ([] comparer: IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - x - else - y - ) - xs +let minBy (projection: 'T -> 'U) (xs: 'T list) ([] comparer: IComparer<'U>) : 'T = + let mutable acc = head xs + let mutable accv = projection acc + let mutable xs = tail xs + + while not xs.IsEmpty do + let x = xs.Head + let v = projection x + + if comparer.Compare(v, accv) < 0 then + acc <- x + accv <- v + + xs <- xs.Tail + + acc let min (xs: 'T list) ([] comparer: IComparer<'T>) : 'T = reduce (fun x y -> - if comparer.Compare(y, x) > 0 then - x - else + if comparer.Compare(y, x) < 0 then y + else + x ) xs diff --git a/src/fable-library-dart/Seq.fs b/src/fable-library-dart/Seq.fs index dc22feb87f..543b5175bc 100644 --- a/src/fable-library-dart/Seq.fs +++ b/src/fable-library-dart/Seq.fs @@ -1114,15 +1114,24 @@ let sum (xs: seq<'T>) ([] adder: IGenericAdder<'T>) : 'T = let sumBy (f: 'T -> 'U) (xs: seq<'T>) ([] adder: IGenericAdder<'U>) : 'U = fold (fun acc x -> adder.Add(acc, f x)) (adder.GetZero()) xs -let maxBy (projection: 'T -> 'U) xs ([] comparer: IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - y - else - x - ) - xs +let maxBy (projection: 'T -> 'U) (xs: seq<'T>) ([] comparer: IComparer<'U>) : 'T = + use e = ofSeq xs + + if not (e.MoveNext()) then + invalidArg "source" SR.inputSequenceEmpty + + let mutable acc = e.Current + let mutable accv = projection acc + + while e.MoveNext() do + let x = e.Current + let v = projection x + + if comparer.Compare(v, accv) > 0 then + acc <- x + accv <- v + + acc let max xs ([] comparer: IComparer<'T>) : 'T = reduce @@ -1134,23 +1143,32 @@ let max xs ([] comparer: IComparer<'T>) : 'T = ) xs -let minBy (projection: 'T -> 'U) xs ([] comparer: IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - x - else - y - ) - xs +let minBy (projection: 'T -> 'U) (xs: seq<'T>) ([] comparer: IComparer<'U>) : 'T = + use e = ofSeq xs + + if not (e.MoveNext()) then + invalidArg "source" SR.inputSequenceEmpty + + let mutable acc = e.Current + let mutable accv = projection acc + + while e.MoveNext() do + let x = e.Current + let v = projection x + + if comparer.Compare(v, accv) < 0 then + acc <- x + accv <- v + + acc let min (xs: seq<'T>) ([] comparer: IComparer<'T>) : 'T = reduce (fun x y -> - if comparer.Compare(y, x) > 0 then - x - else + if comparer.Compare(y, x) < 0 then y + else + x ) xs diff --git a/src/fable-library-dart/Util.dart b/src/fable-library-dart/Util.dart index 16130b60d3..96b0e56d51 100644 --- a/src/fable-library-dart/Util.dart +++ b/src/fable-library-dart/Util.dart @@ -997,7 +997,7 @@ T min(int Function(T, T) comparer, T x, T y) { } T max(int Function(T, T) comparer, T x, T y) { - return comparer(x, y) > 0 ? x : y; + return comparer(x, y) < 0 ? y : x; } T clamp(int Function(T, T) comparer, T value, T min, T max) { diff --git a/tests/Dart/src/ArrayTests.fs b/tests/Dart/src/ArrayTests.fs index b0284149ab..168778db9b 100644 --- a/tests/Dart/src/ArrayTests.fs +++ b/tests/Dart/src/ArrayTests.fs @@ -743,6 +743,19 @@ let tests () = xs |> Array.minBy (fun x -> -x) |> equal 2. + testCase "Array.minBy and Array.maxBy keep the first of tied elements" <| fun () -> + let xs = [| "a", 1; "b", 0; "c", 0; "d", 1 |] + xs |> Array.minBy snd |> equal ("b", 0) + xs |> Array.maxBy snd |> equal ("a", 1) + xs |> Array.minBy (fun _ -> 0) |> equal ("a", 1) + + testCase "Array.minBy and Array.maxBy call the projection once per element" <| fun () -> + let mutable calls = 0 + let f x = calls <- calls + 1; x + [| 5; 3; 8; 1; 9 |] |> Array.minBy f |> equal 1 + [| 5; 3; 8; 1; 9 |] |> Array.maxBy f |> equal 9 + calls |> equal 10 + testCase "Array.ofList works" <| fun () -> let xs = [1.; 2.] let ys = Array.ofList xs diff --git a/tests/Dart/src/ComparisonTests.fs b/tests/Dart/src/ComparisonTests.fs index 763dc8d931..61909191dc 100644 --- a/tests/Dart/src/ComparisonTests.fs +++ b/tests/Dart/src/ComparisonTests.fs @@ -85,6 +85,23 @@ type Status = type MyClass(v) = member val Value: int = v with get, set +type RankedValue(rank: int, label: string) = + member _.Rank = rank + member _.Label = label + + override _.Equals(other) = + match other with + | :? RankedValue as other -> rank = other.Rank + | _ -> false + + override _.GetHashCode() = rank + + interface System.IComparable with + member _.CompareTo(other) = + match other with + | :? RankedValue as other -> compare rank other.Rank + | _ -> invalidArg "other" "cannot compare values of different types" + [] type FuzzyInt = | FuzzyInt of int @@ -390,6 +407,21 @@ let tests() = let c2 = Test(5) Object.ReferenceEquals(min c1 c2, c2) |> equal true + testCase "max and min of equal values return the same argument as .NET" <| fun () -> + let x = RankedValue(0, "x") + let y = RankedValue(0, "y") + (max x y).Label |> equal "x" + (min x y).Label |> equal "y" + + testCase "List, Array and Seq min and max keep the first of equal values" <| fun () -> + let xs = [ RankedValue(1, "a"); RankedValue(0, "b"); RankedValue(0, "c"); RankedValue(1, "d") ] + (List.min xs).Label |> equal "b" + (Array.min (List.toArray xs)).Label |> equal "b" + (Seq.min xs).Label |> equal "b" + (List.max xs).Label |> equal "a" + (Array.max (List.toArray xs)).Label |> equal "a" + (Seq.max xs).Label |> equal "a" + // TODO: More tests, also with longs and decimals testCase "clamp works" <| fun () -> Math.Clamp(14, 0, 12) |> equal 12 diff --git a/tests/Dart/src/ListTests.fs b/tests/Dart/src/ListTests.fs index acdde1fa7d..9e03e9e026 100644 --- a/tests/Dart/src/ListTests.fs +++ b/tests/Dart/src/ListTests.fs @@ -410,6 +410,19 @@ let tests() = xs |> List.minBy (fun x -> -x) |> equal 2 + testCase "List.minBy and List.maxBy keep the first of tied elements" <| fun () -> + let xs = [ "a", 1; "b", 0; "c", 0; "d", 1 ] + xs |> List.minBy snd |> equal ("b", 0) + xs |> List.maxBy snd |> equal ("a", 1) + xs |> List.minBy (fun _ -> 0) |> equal ("a", 1) + + testCase "List.minBy and List.maxBy call the projection once per element" <| fun () -> + let mutable calls = 0 + let f x = calls <- calls + 1; x + [ 5; 3; 8; 1; 9 ] |> List.minBy f |> equal 1 + [ 5; 3; 8; 1; 9 ] |> List.maxBy f |> equal 9 + calls |> equal 10 + testCase "List.sum works" <| fun () -> [1; 2] |> List.sum |> equal 3 diff --git a/tests/Dart/src/SeqTests.fs b/tests/Dart/src/SeqTests.fs index 795c9e297e..1533e6b718 100644 --- a/tests/Dart/src/SeqTests.fs +++ b/tests/Dart/src/SeqTests.fs @@ -401,6 +401,19 @@ let tests() = xs |> Seq.minBy (fun x -> -x) |> equal 2. + testCase "Seq.minBy and Seq.maxBy keep the first of tied elements" <| fun () -> + let xs = Seq.ofList [ "a", 1; "b", 0; "c", 0; "d", 1 ] + xs |> Seq.minBy snd |> equal ("b", 0) + xs |> Seq.maxBy snd |> equal ("a", 1) + xs |> Seq.minBy (fun _ -> 0) |> equal ("a", 1) + + testCase "Seq.minBy and Seq.maxBy call the projection once per element" <| fun () -> + let mutable calls = 0 + let f x = calls <- calls + 1; x + seq { 5; 3; 8; 1; 9 } |> Seq.minBy f |> equal 1 + seq { 5; 3; 8; 1; 9 } |> Seq.maxBy f |> equal 9 + calls |> equal 10 + testCase "Seq.max with non numeric types works" <| fun () -> let p1 = {x=1; y=1} let p2 = {x=2; y=2} From 7914f191f2f38b2f8ed5fbfba0b42b45ac0045ef Mon Sep 17 00:00:00 2001 From: Leo Conforti Date: Tue, 27 Oct 2026 16:22:45 -0600 Subject: [PATCH 4/5] fix(rust): break min and max ties like FSharp.Core - List, Array and Seq minBy, maxBy, min and max returned the last of tied elements; they now return the first, comparing each element with the current one like FSharp.Core. minBy and maxBy call the projection once per element instead of twice per step. - The max operator returned its second argument when the values compare equal; it now returns the first like FSharp.Core. - The decimal max and min operators used Rust's Ord::max and Ord::min, which break ties the other way round from FSharp.Core (max 1.0m 1.00m gave 1.00). They now go through Decimal max and min, which match it. --- src/Fable.Transforms/Rust/Replacements.fs | 1 + src/fable-library-rust/src/Array.fs | 58 ++++++++++++-------- src/fable-library-rust/src/Decimal.rs | 4 +- src/fable-library-rust/src/List.fs | 64 +++++++++++++++-------- src/fable-library-rust/src/Native.rs | 2 +- src/fable-library-rust/src/Seq.fs | 62 ++++++++++++++-------- tests/Rust/tests/src/ArrayTests.fs | 20 +++++++ tests/Rust/tests/src/ComparisonTests.fs | 7 +++ tests/Rust/tests/src/ListTests.fs | 20 +++++++ tests/Rust/tests/src/SeqTests.fs | 20 +++++++ 10 files changed, 189 insertions(+), 69 deletions(-) diff --git a/src/Fable.Transforms/Rust/Replacements.fs b/src/Fable.Transforms/Rust/Replacements.fs index 3e36ebafd7..140f7c7e6f 100644 --- a/src/Fable.Transforms/Rust/Replacements.fs +++ b/src/Fable.Transforms/Rust/Replacements.fs @@ -1253,6 +1253,7 @@ let operators (com: ICompiler) (ctx: Context) r t (i: CallInfo) (thisArg: Expr o | "Clamp", _ -> math r t args i.SignatureArgTypes i.CompiledName |> Some | ("Min" | "Max") as meth, _ -> match args.Head.Type with + | Number(Decimal, _) -> libCall com r t "Decimal" (Naming.lowerFirst meth) args |> Some | Boolean | Char | String diff --git a/src/fable-library-rust/src/Array.fs b/src/fable-library-rust/src/Array.fs index 13c8f44d5b..4abee2a1bb 100644 --- a/src/fable-library-rust/src/Array.fs +++ b/src/fable-library-rust/src/Array.fs @@ -830,42 +830,56 @@ let inline sumBy (projection: 'T -> 'U) (source: 'T[]) : 'U = acc let maxBy (projection: 'T -> 'U) (xs: 'T[]) : 'T = - reduce - (fun x y -> - if (projection x) > (projection y) then - x - else - y - ) - xs + if isEmpty xs then + invalidOp SR.arrayWasEmpty + + let mutable acc = xs[0] + let mutable accv = projection acc + + for i = 1 to xs.Length - 1 do + let x = xs[i] + let v = projection x + + if v > accv then + acc <- x + accv <- v + + acc let max (xs: 'T[]) : 'T = reduce (fun x y -> - if x > y then - x - else + if y > x then y + else + x ) xs let minBy (projection: 'T -> 'U) (xs: 'T[]) : 'T = - reduce - (fun x y -> - if (projection x) < (projection y) then - x - else - y - ) - xs + if isEmpty xs then + invalidOp SR.arrayWasEmpty + + let mutable acc = xs[0] + let mutable accv = projection acc + + for i = 1 to xs.Length - 1 do + let x = xs[i] + let v = projection x + + if v < accv then + acc <- x + accv <- v + + acc let min (xs: 'T[]) : 'T = reduce (fun x y -> - if x < y then - x - else + if y < x then y + else + x ) xs diff --git a/src/fable-library-rust/src/Decimal.rs b/src/fable-library-rust/src/Decimal.rs index 60f521b6a4..168470b7d3 100644 --- a/src/fable-library-rust/src/Decimal.rs +++ b/src/fable-library-rust/src/Decimal.rs @@ -40,8 +40,8 @@ pub mod Decimal_ { if x.is_sign_negative() == sign.is_sign_negative() { x } else { -x } } - pub fn max(x: decimal, y: decimal) -> decimal { x.max(y) } - pub fn min(x: decimal, y: decimal) -> decimal { x.min(y) } + pub fn max(x: decimal, y: decimal) -> decimal { if x < y { y } else { x } } + pub fn min(x: decimal, y: decimal) -> decimal { if x < y { x } else { y } } pub fn maxMagnitude(x: decimal, y: decimal) -> decimal { if abs(x) > abs(y) {x} else {y} } pub fn minMagnitude(x: decimal, y: decimal) -> decimal { if abs(x) < abs(y) {x} else {y} } diff --git a/src/fable-library-rust/src/List.fs b/src/fable-library-rust/src/List.fs index 4ce0353ec0..35ad6fc70e 100644 --- a/src/fable-library-rust/src/List.fs +++ b/src/fable-library-rust/src/List.fs @@ -594,42 +594,62 @@ let inline sumBy (projection: 'T -> 'U) (xs: 'T list) : 'U = fold (fun acc x -> acc + (projection x)) zero xs let maxBy (projection: 'T -> 'U) (xs: 'T list) : 'T = - reduce - (fun x y -> - if (projection x) > (projection y) then - x - else - y - ) - xs + if isEmpty xs then + invalidOp SR.inputListWasEmpty + + let mutable acc = head xs + let mutable accv = projection acc + let mutable xs = tail xs + + while not (isEmpty xs) do + let x = head xs + let v = projection x + + if v > accv then + acc <- x + accv <- v + + xs <- tail xs + + acc let max (xs: 'T list) : 'T = reduce (fun x y -> - if x > y then - x - else + if y > x then y + else + x ) xs let minBy (projection: 'T -> 'U) (xs: 'T list) : 'T = - reduce - (fun x y -> - if (projection x) < (projection y) then - x - else - y - ) - xs + if isEmpty xs then + invalidOp SR.inputListWasEmpty + + let mutable acc = head xs + let mutable accv = projection acc + let mutable xs = tail xs + + while not (isEmpty xs) do + let x = head xs + let v = projection x + + if v < accv then + acc <- x + accv <- v + + xs <- tail xs + + acc let min (xs: 'T list) : 'T = reduce (fun x y -> - if x < y then - x - else + if y < x then y + else + x ) xs diff --git a/src/fable-library-rust/src/Native.rs b/src/fable-library-rust/src/Native.rs index c3ea70f006..2578696b6a 100644 --- a/src/fable-library-rust/src/Native.rs +++ b/src/fable-library-rust/src/Native.rs @@ -348,7 +348,7 @@ pub mod Native_ { } pub fn max(x: T, y: T) -> T { - if x > y { x } else { y } + if y > x { y } else { x } } pub fn equals(x: T, y: T) -> bool { diff --git a/src/fable-library-rust/src/Seq.fs b/src/fable-library-rust/src/Seq.fs index 76c9fbddce..1c055ecd4c 100644 --- a/src/fable-library-rust/src/Seq.fs +++ b/src/fable-library-rust/src/Seq.fs @@ -1233,42 +1233,60 @@ let inline sumBy (projection: 'T -> 'U) (xs: 'T seq) : 'U = fold (fun acc x -> acc + (projection x)) zero xs let maxBy (projection: 'T -> 'U) (xs: 'T seq) : 'T = - reduce - (fun x y -> - if (projection x) > (projection y) then - x - else - y - ) - xs + use e = ofSeq xs + + if e.MoveNext() then + let mutable acc = e.Current + let mutable accv = projection acc + + while e.MoveNext() do + let x = e.Current + let v = projection x + + if v > accv then + acc <- x + accv <- v + + acc + else + invalidOp SR.inputSequenceEmpty let max (xs: 'T seq) : 'T = reduce (fun x y -> - if x > y then - x - else + if y > x then y + else + x ) xs let minBy (projection: 'T -> 'U) (xs: 'T seq) : 'T = - reduce - (fun x y -> - if (projection x) < (projection y) then - x - else - y - ) - xs + use e = ofSeq xs + + if e.MoveNext() then + let mutable acc = e.Current + let mutable accv = projection acc + + while e.MoveNext() do + let x = e.Current + let v = projection x + + if v < accv then + acc <- x + accv <- v + + acc + else + invalidOp SR.inputSequenceEmpty let min (xs: 'T seq) : 'T = reduce (fun x y -> - if x < y then - x - else + if y < x then y + else + x ) xs diff --git a/tests/Rust/tests/src/ArrayTests.fs b/tests/Rust/tests/src/ArrayTests.fs index b860351d83..0e60e87a15 100644 --- a/tests/Rust/tests/src/ArrayTests.fs +++ b/tests/Rust/tests/src/ArrayTests.fs @@ -770,6 +770,26 @@ let ``Array.minBy works`` () = xs |> Array.minBy (fun x -> -x) |> equal 2. +[] +let ``Array.minBy and Array.maxBy keep the first of tied elements`` () = + let xs = [| "a", 1; "b", 0; "c", 0; "d", 1 |] + xs |> Array.minBy snd |> equal ("b", 0) + xs |> Array.maxBy snd |> equal ("a", 1) + xs |> Array.minBy (fun _ -> 0) |> equal ("a", 1) + +[] +let ``Array.minBy and Array.maxBy call the projection once per element`` () = + let mutable calls = 0 + let f (x: int) = calls <- calls + 1; x + [| 5; 3; 8; 1; 9 |] |> Array.minBy f |> equal 1 + [| 5; 3; 8; 1; 9 |] |> Array.maxBy f |> equal 9 + calls |> equal 10 + +[] +let ``Array.min and Array.max keep the first of equal elements`` () = + Array.min [| 1.0m; 1.00m |] |> string |> equal "1.0" + Array.max [| 1.00m; 1.0m |] |> string |> equal "1.00" + [] let ``Array.ofList works`` () = let xs = [1.; 2.] diff --git a/tests/Rust/tests/src/ComparisonTests.fs b/tests/Rust/tests/src/ComparisonTests.fs index d75e826b9b..ff1f211486 100644 --- a/tests/Rust/tests/src/ComparisonTests.fs +++ b/tests/Rust/tests/src/ComparisonTests.fs @@ -421,6 +421,13 @@ let ``min works with records`` () = let r2 = {a=1; b=2} min r1 r2 |> equal r1 +[] +let ``max and min of equal values return the same argument as .NET`` () = + max 1.0m 1.00m |> string |> equal "1.0" + min 1.0m 1.00m |> string |> equal "1.00" + max (1.0m, 0) (1.00m, 0) |> fst |> string |> equal "1.0" + min (1.0m, 0) (1.00m, 0) |> fst |> string |> equal "1.00" + // [] // let ``min with objects implementing IComparable works`` () = // let c1 = Test(5) diff --git a/tests/Rust/tests/src/ListTests.fs b/tests/Rust/tests/src/ListTests.fs index 456d27e883..ee6f807358 100644 --- a/tests/Rust/tests/src/ListTests.fs +++ b/tests/Rust/tests/src/ListTests.fs @@ -563,6 +563,26 @@ let ``List.minBy works`` () = xs |> List.minBy (fun x -> -x) |> equal 2 +[] +let ``List.minBy and List.maxBy keep the first of tied elements`` () = + let xs = [ "a", 1; "b", 0; "c", 0; "d", 1 ] + xs |> List.minBy snd |> equal ("b", 0) + xs |> List.maxBy snd |> equal ("a", 1) + xs |> List.minBy (fun _ -> 0) |> equal ("a", 1) + +[] +let ``List.minBy and List.maxBy call the projection once per element`` () = + let mutable calls = 0 + let f (x: int) = calls <- calls + 1; x + [ 5; 3; 8; 1; 9 ] |> List.minBy f |> equal 1 + [ 5; 3; 8; 1; 9 ] |> List.maxBy f |> equal 9 + calls |> equal 10 + +[] +let ``List.min and List.max keep the first of equal elements`` () = + List.min [ 1.0m; 1.00m ] |> string |> equal "1.0" + List.max [ 1.00m; 1.0m ] |> string |> equal "1.00" + [] let ``List.sum works`` () = [1; 2] |> List.sum diff --git a/tests/Rust/tests/src/SeqTests.fs b/tests/Rust/tests/src/SeqTests.fs index 5f07d3d78e..3b04b89f3b 100644 --- a/tests/Rust/tests/src/SeqTests.fs +++ b/tests/Rust/tests/src/SeqTests.fs @@ -540,6 +540,26 @@ let ``Seq.minBy works`` () = xs |> Seq.minBy (fun x -> -x) |> equal 2. +[] +let ``Seq.minBy and Seq.maxBy keep the first of tied elements`` () = + let xs = Seq.ofList [ "a", 1; "b", 0; "c", 0; "d", 1 ] + xs |> Seq.minBy snd |> equal ("b", 0) + xs |> Seq.maxBy snd |> equal ("a", 1) + xs |> Seq.minBy (fun _ -> 0) |> equal ("a", 1) + +[] +let ``Seq.minBy and Seq.maxBy call the projection once per element`` () = + let mutable calls = 0 + let f (x: int) = calls <- calls + 1; x + seq { 5; 3; 8; 1; 9 } |> Seq.minBy f |> equal 1 + seq { 5; 3; 8; 1; 9 } |> Seq.maxBy f |> equal 9 + calls |> equal 10 + +[] +let ``Seq.min and Seq.max keep the first of equal elements`` () = + Seq.min (seq { 1.0m; 1.00m }) |> string |> equal "1.0" + Seq.max (seq { 1.00m; 1.0m }) |> string |> equal "1.00" + [] let ``Seq.max with non numeric types works`` () = let p1 = {x=1; y=1} From 00e8a074fc4fca6356541b9caa0a7f6e75764fce Mon Sep 17 00:00:00 2001 From: Leo Conforti Date: Tue, 27 Oct 2026 16:24:36 -0600 Subject: [PATCH 5/5] fix(beam): break min and max ties like FSharp.Core - List and Array minBy and maxBy broke key ties by comparing the elements themselves in Erlang term order, so they returned the smallest or largest of the tied elements. They now return the first, as they already did for union keys. - Seq minBy returned the last of the tied minima, and Seq min the last of equal elements; they now return the first. Seq minBy and maxBy call the projection once per element instead of twice per step. --- src/fable-library-beam/Seq.fs | 60 +++++++++++++++++---------- src/fable-library-beam/fable_list.erl | 30 ++++++++++++-- tests/Beam/ArrayTests.fs | 16 +++++++ tests/Beam/ListTests.fs | 16 +++++++ tests/Beam/SeqTests.fs | 16 +++++++ 5 files changed, 113 insertions(+), 25 deletions(-) diff --git a/src/fable-library-beam/Seq.fs b/src/fable-library-beam/Seq.fs index 71671d5394..3ce141dbc0 100644 --- a/src/fable-library-beam/Seq.fs +++ b/src/fable-library-beam/Seq.fs @@ -1166,15 +1166,24 @@ let sum (xs: seq<'T>) ([] adder: IGenericAdder<'T>) : 'T = let sumBy (f: 'T -> 'U) (xs: seq<'T>) ([] adder: IGenericAdder<'U>) : 'U = fold (fun acc x -> adder.Add(acc, f x)) (adder.GetZero()) xs -let maxBy (projection: 'T -> 'U) xs ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - y - else - x - ) - xs +let maxBy (projection: 'T -> 'U) (xs: seq<'T>) ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = + use e = ofSeq xs + + if not (e.MoveNext()) then + invalidArg "source" SR.inputSequenceEmpty + + let mutable acc = e.Current + let mutable accv = projection acc + + while e.MoveNext() do + let x = e.Current + let v = projection x + + if comparer.Compare(v, accv) > 0 then + acc <- x + accv <- v + + acc let max xs ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T = reduce @@ -1186,23 +1195,32 @@ let max xs ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T ) xs -let minBy (projection: 'T -> 'U) xs ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = - reduce - (fun x y -> - if comparer.Compare(projection y, projection x) > 0 then - x - else - y - ) - xs +let minBy (projection: 'T -> 'U) (xs: seq<'T>) ([] comparer: System.Collections.Generic.IComparer<'U>) : 'T = + use e = ofSeq xs + + if not (e.MoveNext()) then + invalidArg "source" SR.inputSequenceEmpty + + let mutable acc = e.Current + let mutable accv = projection acc + + while e.MoveNext() do + let x = e.Current + let v = projection x + + if comparer.Compare(v, accv) < 0 then + acc <- x + accv <- v + + acc let min (xs: seq<'T>) ([] comparer: System.Collections.Generic.IComparer<'T>) : 'T = reduce (fun x y -> - if comparer.Compare(y, x) > 0 then - x - else + if comparer.Compare(y, x) < 0 then y + else + x ) xs diff --git a/src/fable-library-beam/fable_list.erl b/src/fable-library-beam/fable_list.erl index 580eb7c9e5..caac01f6fa 100644 --- a/src/fable-library-beam/fable_list.erl +++ b/src/fable-library-beam/fable_list.erl @@ -286,8 +286,19 @@ collect(Fn, List) -> sum_by(Fn, List) -> lists:sum(lists:map(Fn, List)). -min_by(Fn, List) -> - element(2, lists:min(lists:map(fun(E) -> {Fn(E), E} end, List))). +min_by(Fn, [Head | Tail]) -> + {_, Result} = lists:foldl( + fun(Item, {BestKey, Best}) -> + ItemKey = Fn(Item), + case ItemKey < BestKey of + true -> {ItemKey, Item}; + false -> {BestKey, Best} + end + end, + {Fn(Head), Head}, + Tail + ), + Result. min_by(Fn, Comparer, [Head | Tail]) -> {_, Result} = lists:foldl( @@ -303,8 +314,19 @@ min_by(Fn, Comparer, [Head | Tail]) -> ), Result. -max_by(Fn, List) -> - element(2, lists:max(lists:map(fun(E) -> {Fn(E), E} end, List))). +max_by(Fn, [Head | Tail]) -> + {_, Result} = lists:foldl( + fun(Item, {BestKey, Best}) -> + ItemKey = Fn(Item), + case ItemKey > BestKey of + true -> {ItemKey, Item}; + false -> {BestKey, Best} + end + end, + {Fn(Head), Head}, + Tail + ), + Result. max_by(Fn, Comparer, [Head | Tail]) -> {_, Result} = lists:foldl( diff --git a/tests/Beam/ArrayTests.fs b/tests/Beam/ArrayTests.fs index 36cc63eb54..f075daf7a3 100644 --- a/tests/Beam/ArrayTests.fs +++ b/tests/Beam/ArrayTests.fs @@ -408,6 +408,22 @@ let ``test Array.minBy works`` () = xs |> Array.minBy (fun x -> -x) |> equal 2. +[] +let ``test Array.minBy and Array.maxBy keep the first of tied elements`` () = + let xs = [| "y", 1; "c", 0; "a", 0; "z", 1; "b", 0; "x", 1 |] + xs |> Array.minBy snd |> equal ("c", 0) + xs |> Array.maxBy snd |> equal ("y", 1) + xs |> Array.minBy (fun _ -> 0) |> equal ("y", 1) + xs |> Array.maxBy (fun _ -> 0) |> equal ("y", 1) + +[] +let ``test Array.minBy and Array.maxBy call the projection once per element`` () = + let mutable calls = 0 + let f (x: int) = calls <- calls + 1; x + [| 5; 3; 8; 1; 9 |] |> Array.minBy f |> equal 1 + [| 5; 3; 8; 1; 9 |] |> Array.maxBy f |> equal 9 + calls |> equal 10 + [] let ``test Array.max works`` () = let xs = [| 1.; 2. |] diff --git a/tests/Beam/ListTests.fs b/tests/Beam/ListTests.fs index 498beca6e1..dfe128747c 100644 --- a/tests/Beam/ListTests.fs +++ b/tests/Beam/ListTests.fs @@ -230,6 +230,22 @@ let ``test List.minBy works`` () = let ``test List.maxBy works`` () = ["ab"; "a"; "abc"] |> List.maxBy (fun s -> s.Length) |> equal "abc" +[] +let ``test List.minBy and List.maxBy keep the first of tied elements`` () = + let xs = [ "y", 1; "c", 0; "a", 0; "z", 1; "b", 0; "x", 1 ] + xs |> List.minBy snd |> equal ("c", 0) + xs |> List.maxBy snd |> equal ("y", 1) + xs |> List.minBy (fun _ -> 0) |> equal ("y", 1) + xs |> List.maxBy (fun _ -> 0) |> equal ("y", 1) + +[] +let ``test List.minBy and List.maxBy call the projection once per element`` () = + let mutable calls = 0 + let f (x: int) = calls <- calls + 1; x + [ 5; 3; 8; 1; 9 ] |> List.minBy f |> equal 1 + [ 5; 3; 8; 1; 9 ] |> List.maxBy f |> equal 9 + calls |> equal 10 + [] let ``test List.init works`` () = List.init 5 (fun i -> i * 2) |> equal [0; 2; 4; 6; 8] diff --git a/tests/Beam/SeqTests.fs b/tests/Beam/SeqTests.fs index ea637949d3..ed7fbc2b49 100644 --- a/tests/Beam/SeqTests.fs +++ b/tests/Beam/SeqTests.fs @@ -411,6 +411,22 @@ let ``test Seq.maxBy works`` () = let ``test Seq.minBy works`` () = [1; 3; 2] |> Seq.minBy (fun x -> -x) |> equal 3 +[] +let ``test Seq.minBy and Seq.maxBy keep the first of tied elements`` () = + let xs = Seq.ofList [ "y", 1; "c", 0; "a", 0; "z", 1; "b", 0; "x", 1 ] + xs |> Seq.minBy snd |> equal ("c", 0) + xs |> Seq.maxBy snd |> equal ("y", 1) + xs |> Seq.minBy (fun _ -> 0) |> equal ("y", 1) + xs |> Seq.maxBy (fun _ -> 0) |> equal ("y", 1) + +[] +let ``test Seq.minBy and Seq.maxBy call the projection once per element`` () = + let mutable calls = 0 + let f (x: int) = calls <- calls + 1; x + seq { 5; 3; 8; 1; 9 } |> Seq.minBy f |> equal 1 + seq { 5; 3; 8; 1; 9 } |> Seq.maxBy f |> equal 9 + calls |> equal 10 + [] let ``test Seq.ofArray works`` () = [| 1; 2; 3 |] |> Seq.ofArray |> Seq.toList |> equal [1; 2; 3]