Skip to content

Commit bbaaf54

Browse files
committed
Fix findmax and findmin with iterables
The state is not guaranteed to be equivalent to the index of the element.
1 parent 3da9aee commit bbaaf54

File tree

4 files changed

+28
-23
lines changed

4 files changed

+28
-23
lines changed

base/abstractarray.jl

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -400,9 +400,6 @@ start(A::AbstractArray) = (@_inline_meta(); itr = eachindex(A); (itr, start(itr)
400400
next(A::AbstractArray,i) = (@_inline_meta(); (idx, s) = next(i[1], i[2]); (A[idx], (i[1], s)))
401401
done(A::AbstractArray,i) = done(i[1], i[2])
402402

403-
iterstate(i) = i
404-
iterstate(i::Tuple{UnitRange{Int},Int}) = i[2]
405-
406403
# eachindex iterates over all indices. LinearSlow definitions are later.
407404
eachindex(A::AbstractArray) = (@_inline_meta(); eachindex(linearindexing(A), A))
408405

base/array.jl

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -821,36 +821,36 @@ function findmax(a)
821821
if isempty(a)
822822
throw(ArgumentError("collection must be non-empty"))
823823
end
824-
i = start(a)
825-
mi = i
826-
m, i = next(a, i)
827-
while !done(a, i)
828-
iold = i
829-
ai, i = next(a, i)
824+
s = start(a)
825+
mi = i = 1
826+
m, s = next(a, s)
827+
while !done(a, s)
828+
ai, s = next(a, s)
829+
i += 1
830830
if ai > m || m!=m
831831
m = ai
832-
mi = iold
832+
mi = i
833833
end
834834
end
835-
return (m, iterstate(mi))
835+
return (m, mi)
836836
end
837837

838838
function findmin(a)
839839
if isempty(a)
840840
throw(ArgumentError("collection must be non-empty"))
841841
end
842-
i = start(a)
843-
mi = i
844-
m, i = next(a, i)
845-
while !done(a, i)
846-
iold = i
847-
ai, i = next(a, i)
842+
s = start(a)
843+
mi = i = 1
844+
m, s = next(a, s)
845+
while !done(a, s)
846+
ai, s = next(a, s)
847+
i += 1
848848
if ai < m || m!=m
849849
m = ai
850-
mi = iold
850+
mi = i
851851
end
852852
end
853-
return (m, iterstate(mi))
853+
return (m, mi)
854854
end
855855

856856
indmax(a) = findmax(a)[2]

base/multidimensional.jl

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
### Multidimensional iterators
44
module IteratorsMD
55

6-
import Base: eltype, length, start, done, next, last, getindex, setindex!, linearindexing, min, max, eachindex, ndims, iterstate
6+
import Base: eltype, length, start, done, next, last, getindex, setindex!, linearindexing, min, max, eachindex, ndims
77
importall ..Base.Operators
88
import Base: simd_outer_range, simd_inner_length, simd_index, @generated
99
import Base: @nref, @ncall, @nif, @nexprs, LinearFast, LinearSlow, to_index
@@ -59,8 +59,6 @@ immutable CartesianRange{I<:CartesianIndex}
5959
stop::I
6060
end
6161

62-
iterstate{CR<:CartesianRange,CI<:CartesianIndex}(i::Tuple{CR,CI}) = Base._sub2ind(i[1].stop.I, i[2].I)
63-
6462
@generated function CartesianRange{N}(I::CartesianIndex{N})
6563
startargs = fill(1, N)
6664
:(CartesianRange($I($(startargs...)), I))

test/arrayops.jl

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ for i = 1:3
306306
end
307307
@test isequal(a,findn(z))
308308

309-
#argmin argmax
309+
#findmin findmax indmin indmax
310310
@test indmax([10,12,9,11]) == 2
311311
@test indmin([10,12,9,11]) == 3
312312
@test findmin([NaN,3.2,1.8]) == (1.8,3)
@@ -316,6 +316,16 @@ end
316316
@test findmin([3.2,1.8,NaN,2.0]) == (1.8,2)
317317
@test findmax([3.2,1.8,NaN,2.0]) == (3.2,1)
318318

319+
# #14085
320+
@test findmax(4:9) == (9,6)
321+
@test indmax(4:9) == 6
322+
@test findmin(4:9) == (4,1)
323+
@test indmin(4:9) == 1
324+
@test findmax(5:-2:1) == (5,1)
325+
@test indmax(5:-2:1) == 1
326+
@test findmin(5:-2:1) == (1,3)
327+
@test indmin(5:-2:1) == 3
328+
319329
## permutedims ##
320330

321331
#keeps the num of dim

0 commit comments

Comments
 (0)