Skip to content

Commit

Permalink
min, max, minmax use isless for comparison (fix #23094) (#23155)
Browse files Browse the repository at this point in the history
  • Loading branch information
KlausC authored and JeffBezanson committed Aug 9, 2017
1 parent cdb660c commit 5f582ae
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 7 deletions.
10 changes: 6 additions & 4 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1697,7 +1697,7 @@ end
Returns the maximum element of the collection `itr` and its index. If there are multiple
maximal elements, then the first one will be returned. `NaN` values are ignored, unless
all elements are `NaN`.
all elements are `NaN`. Other than the treatment of `NaN`, the result is in line with `max`.
The collection must not be empty.
Expand All @@ -1723,7 +1723,8 @@ function findmax(a)
while !done(a, s)
ai, s = next(a, s)
i += 1
if ai > m || m!=m
ai != ai && continue # assume x != x => x is a NaN
if m != m || isless(m, ai)
m = ai
mi = i
end
Expand All @@ -1736,7 +1737,7 @@ end
Returns the minimum element of the collection `itr` and its index. If there are multiple
minimal elements, then the first one will be returned. `NaN` values are ignored, unless
all elements are `NaN`.
all elements are `NaN`. Other than the treatment of `NaN`, the result is in line with `min`.
The collection must not be empty.
Expand All @@ -1762,7 +1763,8 @@ function findmin(a)
while !done(a, s)
ai, s = next(a, s)
i += 1
if ai < m || m!=m
ai != ai && continue
if m != m || isless(ai, m)
m = ai
mi = i
end
Expand Down
6 changes: 3 additions & 3 deletions base/operators.jl
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ julia> max(2, 5, 1)
5
```
"""
max(x, y) = ifelse(y < x, x, y)
max(x, y) = ifelse(isless(y, x), x, y)

"""
min(x, y, ...)
Expand All @@ -373,7 +373,7 @@ julia> min(2, 5, 1)
1
```
"""
min(x,y) = ifelse(y < x, y, x)
min(x,y) = ifelse(isless(y, x), y, x)

"""
minmax(x, y)
Expand All @@ -386,7 +386,7 @@ julia> minmax('c','b')
('b', 'c')
```
"""
minmax(x,y) = y < x ? (y, x) : (x, y)
minmax(x,y) = isless(y, x) ? (y, x) : (x, y)

scalarmax(x,y) = max(x,y)
scalarmax(x::AbstractArray, y::AbstractArray) = throw(ArgumentError("ordering is not well-defined for arrays"))
Expand Down
14 changes: 14 additions & 0 deletions test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,20 @@ end
@test indmax(5:-2:1) == 1
@test findmin(5:-2:1) == (1,3)
@test indmin(5:-2:1) == 3

#23094
@test findmax(Set(["abc"])) === ("abc", 1)
@test findmin(["abc", "a"]) === ("a", 2)
@test_throws MethodError findmax([Set([1]), Set([2])])
@test findmin([0.0, -0.0]) === (-0.0, 2)
@test findmax([0.0, -0.0]) === (0.0, 1)
@test findmin([-0.0, 0.0]) === (-0.0, 1)
@test findmax([-0.0, 0.0]) === (0.0, 2)
@test isnan(findmin([NaN, NaN, 0.0/0.0])[1])
@test findmin([NaN, NaN, 0.0/0.0])[2] == 1
@test isnan(findmax([NaN, NaN, 0.0/0.0])[1])
@test findmax([NaN, NaN, 0.0/0.0])[2] == 1

end

@testset "permutedims" begin
Expand Down
23 changes: 23 additions & 0 deletions test/operators.jl
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,29 @@ p = 1=>:foo
@test_throws ArgumentError Base.scalarmax('a',['c','d'])
@test_throws ArgumentError Base.scalarmax(['a','b'],'c')

@test_throws MethodError min(Set([1]), Set([2]))
@test_throws MethodError max(Set([1]), Set([2]))
@test_throws MethodError minmax(Set([1]), Set([2]))

# Test if isless (not <) is used by min, max, minmax
# and commutativity.
struct TO23094
x::Int
end
Base.isless(a::TO23094, b::TO23094) = isless(a.x, b.x)
Base.isequal(a::TO23094, b::TO23094) = isequal(a.x, b.x)
import Base.<
<(a::TO23094, b::TO23094) = error("< should not be called")

@test isequal(min(TO23094(1), TO23094(2)), TO23094(1))
@test isequal(min(TO23094(2), TO23094(1)), TO23094(1))
@test isequal(max(TO23094(1), TO23094(2)), TO23094(2))
@test isequal(max(TO23094(2), TO23094(1)), TO23094(2))
@test isequal(minmax(TO23094(1), TO23094(2))[1], TO23094(1))
@test isequal(minmax(TO23094(1), TO23094(2))[2], TO23094(2))
@test isequal(minmax(TO23094(2), TO23094(1))[1], TO23094(1))
@test isequal(minmax(TO23094(2), TO23094(1))[2], TO23094(2))

@test lexless('a','b')

@test 1 .!= 2
Expand Down

0 comments on commit 5f582ae

Please sign in to comment.