use pairs in findmin and findmax, supporting all indexable collections

return `CartesianIndex` for n-d arrays in findmin, findmax, indmin, indmax

more compact printing of `CartesianIndex`

change sparse `_findr` macro to a function

Author: JeffBezanson

NEWS.md

@@ -191,6 +191,10 @@ This section lists changes that do not have deprecation warnings.
     This avoids stack overflows in the common case of definitions like
     `f(x, y) = f(promote(x, y)...)` ([#22801]).
 
+  * `findmin`, `findmax`, `indmin`, and `indmax` used to always return linear indices.
+    They now return `CartesianIndex`es for all but 1-d arrays, and in general return
+    the `keys` of indexed collections (e.g. dictionaries) ([#22907]).
+
 Library improvements
 --------------------
 

base/array.jl

@@ -2072,13 +2072,13 @@ function findmax(a)
     if isempty(a)
         throw(ArgumentError("collection must be non-empty"))
     end
-    s = start(a)
-    mi = i = 1
-    m, s = next(a, s)
-    while !done(a, s)
+    p = pairs(a)
+    s = start(p)
+    (mi, m), s = next(p, s)
+    i = mi
+    while !done(p, s)
         m != m && break
-        ai, s = next(a, s)
-        i += 1
+        (i, ai), s = next(p, s)
         if ai != ai || isless(m, ai)
             m = ai
             mi = i
@@ -2113,13 +2113,13 @@ function findmin(a)
     if isempty(a)
         throw(ArgumentError("collection must be non-empty"))
     end
-    s = start(a)
-    mi = i = 1
-    m, s = next(a, s)
-    while !done(a, s)
+    p = pairs(a)
+    s = start(p)
+    (mi, m), s = next(p, s)
+    i = mi
+    while !done(p, s)
         m != m && break
-        ai, s = next(a, s)
-        i += 1
+        (i, ai), s = next(p, s)
         if ai != ai || isless(ai, m)
             m = ai
             mi = i

base/multidimensional.jl

@@ -4,7 +4,7 @@
 module IteratorsMD
     import Base: eltype, length, size, start, done, next, first, last, in, getindex,
                  setindex!, IndexStyle, min, max, zero, one, isless, eachindex,
-                 ndims, iteratorsize, convert
+                 ndims, iteratorsize, convert, show
 
     import Base: +, -, *
     import Base: simd_outer_range, simd_inner_length, simd_index
@@ -80,6 +80,7 @@ module IteratorsMD
     @inline _flatten(i, I...)                 = (i, _flatten(I...)...)
     @inline _flatten(i::CartesianIndex, I...) = (i.I..., _flatten(I...)...)
     CartesianIndex(index::Tuple{Vararg{Union{Integer, CartesianIndex}}}) = CartesianIndex(index...)
+    show(io::IO, i::CartesianIndex) = (print(io, "CartesianIndex"); show(io, i.I))
 
     # length
     length(::CartesianIndex{N}) where {N} = N

base/reducedim.jl

@@ -635,20 +635,22 @@ function findminmax!(f, Rval, Rind, A::AbstractArray{T,N}) where {T,N}
     # Otherwise, keep the result in Rval/Rind so that we traverse A in storage order.
     indsAt, indsRt = safe_tail(indices(A)), safe_tail(indices(Rval))
     keep, Idefault = Broadcast.shapeindexer(indsAt, indsRt)
-    k = 0
+    ks = keys(A)
+    k, kss = next(ks, start(ks))
+    zi = zero(eltype(ks))
     if reducedim1(Rval, A)
         i1 = first(indices1(Rval))
         @inbounds for IA in CartesianRange(indsAt)
             IR = Broadcast.newindex(IA, keep, Idefault)
             tmpRv = Rval[i1,IR]
             tmpRi = Rind[i1,IR]
             for i in indices(A,1)
-                k += 1
                 tmpAv = A[i,IA]
-                if tmpRi == 0 || (tmpRv == tmpRv && (tmpAv != tmpAv || f(tmpAv, tmpRv)))
+                if tmpRi == zi || (tmpRv == tmpRv && (tmpAv != tmpAv || f(tmpAv, tmpRv)))
                     tmpRv = tmpAv
                     tmpRi = k
                 end
+                k, kss = next(ks, kss)
             end
             Rval[i1,IR] = tmpRv
             Rind[i1,IR] = tmpRi
@@ -657,14 +659,14 @@ function findminmax!(f, Rval, Rind, A::AbstractArray{T,N}) where {T,N}
         @inbounds for IA in CartesianRange(indsAt)
             IR = Broadcast.newindex(IA, keep, Idefault)
             for i in indices(A, 1)
-                k += 1
                 tmpAv = A[i,IA]
                 tmpRv = Rval[i,IR]
                 tmpRi = Rind[i,IR]
-                if tmpRi == 0 || (tmpRv == tmpRv && (tmpAv != tmpAv || f(tmpAv, tmpRv)))
+                if tmpRi == zi || (tmpRv == tmpRv && (tmpAv != tmpAv || f(tmpAv, tmpRv)))
                     Rval[i,IR] = tmpAv
                     Rind[i,IR] = k
                 end
+                k, kss = next(ks, kss)
             end
         end
     end
@@ -680,7 +682,7 @@ dimensions of `rval` and `rind`, and store the results in `rval` and `rind`.
 """
 function findmin!(rval::AbstractArray, rind::AbstractArray, A::AbstractArray;
                   init::Bool=true)
-    findminmax!(isless, init && !isempty(A) ? fill!(rval, first(A)) : rval, fill!(rind,0), A)
+    findminmax!(isless, init && !isempty(A) ? fill!(rval, first(A)) : rval, fill!(rind,zero(eltype(keys(A)))), A)
 end
 
 """
@@ -709,10 +711,10 @@ function findmin(A::AbstractArray{T}, region) where T
         if prod(map(length, reduced_indices(A, region))) != 0
             throw(ArgumentError("collection slices must be non-empty"))
         end
-        (similar(A, ri), similar(dims->zeros(Int, dims), ri))
+        (similar(A, ri), similar(dims->zeros(eltype(keys(A)), dims), ri))
     else
         findminmax!(isless, fill!(similar(A, ri), first(A)),
-                    similar(dims->zeros(Int, dims), ri), A)
+                    similar(dims->zeros(eltype(keys(A)), dims), ri), A)
     end
 end
 
@@ -727,7 +729,7 @@ dimensions of `rval` and `rind`, and store the results in `rval` and `rind`.
 """
 function findmax!(rval::AbstractArray, rind::AbstractArray, A::AbstractArray;
                   init::Bool=true)
-    findminmax!(isgreater, init && !isempty(A) ? fill!(rval, first(A)) : rval, fill!(rind,0), A)
+    findminmax!(isgreater, init && !isempty(A) ? fill!(rval, first(A)) : rval, fill!(rind,zero(eltype(keys(A)))), A)
 end
 
 """
@@ -756,10 +758,10 @@ function findmax(A::AbstractArray{T}, region) where T
         if prod(map(length, reduced_indices(A, region))) != 0
             throw(ArgumentError("collection slices must be non-empty"))
         end
-        similar(A, ri), similar(dims->zeros(Int, dims), ri)
+        similar(A, ri), similar(dims->zeros(eltype(keys(A)), dims), ri)
     else
         findminmax!(isgreater, fill!(similar(A, ri), first(A)),
-                    similar(dims->zeros(Int, dims), ri), A)
+                    similar(dims->zeros(eltype(keys(A)), dims), ri), A)
     end
 end
 

base/sparse/sparsematrix.jl

@@ -1815,101 +1815,100 @@ function _findz(A::SparseMatrixCSC{Tv,Ti}, rows=1:A.m, cols=1:A.n) where {Tv,Ti}
     row = 0
     rowmin = rows[1]; rowmax = rows[end]
     allrows = (rows == 1:A.m)
-    @inbounds for  col in cols
+    @inbounds for col in cols
         r1::Int = colptr[col]
         r2::Int = colptr[col+1] - 1
         if !allrows && (r1 <= r2)
             r1 = searchsortedfirst(rowval, rowmin, r1, r2, Forward)
             (r1 <= r2 ) && (r2 = searchsortedlast(rowval, rowmax, r1, r2, Forward))
         end
         row = rowmin
-
         while (r1 <= r2) && (row == rowval[r1]) && (nzval[r1] != zval)
             r1 += 1
             row += 1
         end
-        (row <= rowmax) && (return sub2ind(size(A), row, col))
+        (row <= rowmax) && (return CartesianIndex(row, col))
     end
-    return 0
+    return CartesianIndex(0, 0)
 end
 
-macro _findr(op, A, region, Tv, Ti)
-    esc(quote
-    N = nnz($A)
-    L = length($A)
+function _findr(op, A, region, Tv)
+    Ti = eltype(keys(A))
+    i1 = first(keys(A))
+    N = nnz(A)
+    L = length(A)
     if L == 0
-        if prod(map(length, Base.reduced_indices($A, $region))) != 0
+        if prod(map(length, Base.reduced_indices(A, region))) != 0
             throw(ArgumentError("array slices must be non-empty"))
         else
-            ri = Base.reduced_indices0($A, $region)
-            return (similar($A, ri), similar(dims->zeros(Int, dims), ri))
+            ri = Base.reduced_indices0(A, region)
+            return (similar(A, ri), similar(dims->zeros(Ti, dims), ri))
         end
     end
 
-    colptr = $A.colptr; rowval = $A.rowval; nzval = $A.nzval; m = $A.m; n = $A.n
-    zval = zero($Tv)
-    szA = size($A)
+    colptr = A.colptr; rowval = A.rowval; nzval = A.nzval; m = A.m; n = A.n
+    zval = zero(Tv)
+    szA = size(A)
 
-    if $region == 1 || $region == (1,)
-        (N == 0) && (return (fill(zval,1,n), fill(convert($Ti,1),1,n)))
-        S = Vector{$Tv}(n); I = Vector{$Ti}(n)
+    if region == 1 || region == (1,)
+        (N == 0) && (return (fill(zval,1,n), fill(i1,1,n)))
+        S = Vector{Tv}(n); I = Vector{Ti}(n)
         @inbounds for i = 1 : n
-            Sc = zval; Ic = _findz($A, 1:m, i:i)
-            if Ic == 0
+            Sc = zval; Ic = _findz(A, 1:m, i:i)
+            if Ic == CartesianIndex(0, 0)
                 j = colptr[i]
-                Ic = sub2ind(szA, rowval[j], i)
+                Ic = CartesianIndex(rowval[j], i)
                 Sc = nzval[j]
             end
             for j = colptr[i] : colptr[i+1]-1
-                if ($op)(nzval[j], Sc)
+                if op(nzval[j], Sc)
                     Sc = nzval[j]
-                    Ic = sub2ind(szA, rowval[j], i)
+                    Ic = CartesianIndex(rowval[j], i)
                 end
             end
             S[i] = Sc; I[i] = Ic
         end
         return(reshape(S,1,n), reshape(I,1,n))
-    elseif $region == 2 || $region == (2,)
-        (N == 0) && (return (fill(zval,m,1), fill(convert($Ti,1),m,1)))
-        S = Vector{$Tv}(m); I = Vector{$Ti}(m)
+    elseif region == 2 || region == (2,)
+        (N == 0) && (return (fill(zval,m,1), fill(i1,m,1)))
+        S = Vector{Tv}(m); I = Vector{Ti}(m)
         @inbounds for row in 1:m
-            S[row] = zval; I[row] = _findz($A, row:row, 1:n)
-            if I[row] == 0
-                I[row] = sub2ind(szA, row, 1)
+            S[row] = zval; I[row] = _findz(A, row:row, 1:n)
+            if I[row] == CartesianIndex(0, 0)
+                I[row] = CartesianIndex(row, 1)
                 S[row] = A[row,1]
             end
         end
         @inbounds for i = 1 : n, j = colptr[i] : colptr[i+1]-1
             row = rowval[j]
-            if ($op)(nzval[j], S[row])
+            if op(nzval[j], S[row])
                 S[row] = nzval[j]
-                I[row] = sub2ind(szA, row, i)
+                I[row] = CartesianIndex(row, i)
             end
         end
         return (reshape(S,m,1), reshape(I,m,1))
-    elseif $region == (1,2)
-        (N == 0) && (return (fill(zval,1,1), fill(convert($Ti,1),1,1)))
-        hasz = nnz($A) != length($A)
+    elseif region == (1,2)
+        (N == 0) && (return (fill(zval,1,1), fill(i1,1,1)))
+        hasz = nnz(A) != length(A)
         Sv = hasz ? zval : nzval[1]
-        Iv::($Ti) = hasz ? _findz($A) : 1
-        @inbounds for i = 1 : $A.n, j = colptr[i] : (colptr[i+1]-1)
-            if ($op)(nzval[j], Sv)
+        Iv::(Ti) = hasz ? _findz(A) : i1
+        @inbounds for i = 1 : A.n, j = colptr[i] : (colptr[i+1]-1)
+            if op(nzval[j], Sv)
                 Sv = nzval[j]
-                Iv = sub2ind(szA, rowval[j], i)
+                Iv = CartesianIndex(rowval[j], i)
             end
         end
         return (fill(Sv,1,1), fill(Iv,1,1))
     else
         throw(ArgumentError("invalid value for region; must be 1, 2, or (1,2)"))
     end
-    end) #quote
 end
 
 _isless_fm(a, b)    =  b == b && ( a != a || isless(a, b) )
 _isgreater_fm(a, b) =  b == b && ( a != a || isless(b, a) )
 
-findmin(A::SparseMatrixCSC{Tv,Ti}, region) where {Tv,Ti} = @_findr(_isless_fm, A, region, Tv, Ti)
-findmax(A::SparseMatrixCSC{Tv,Ti}, region) where {Tv,Ti} = @_findr(_isgreater_fm, A, region, Tv, Ti)
+findmin(A::SparseMatrixCSC{Tv,Ti}, region) where {Tv,Ti} = _findr(_isless_fm, A, region, Tv)
+findmax(A::SparseMatrixCSC{Tv,Ti}, region) where {Tv,Ti} = _findr(_isgreater_fm, A, region, Tv)
 findmin(A::SparseMatrixCSC) = (r=findmin(A,(1,2)); (r[1][1], r[2][1]))
 findmax(A::SparseMatrixCSC) = (r=findmax(A,(1,2)); (r[1][1], r[2][1]))
 

test/arrayops.jl

@@ -503,7 +503,7 @@ end
     @test indmin(5:-2:1) == 3
 
     #23094
-    @test findmax(Set(["abc"])) === ("abc", 1)
+    @test_throws MethodError findmax(Set(["abc"]))
     @test findmin(["abc", "a"]) === ("a", 2)
     @test_throws MethodError findmax([Set([1]), Set([2])])
     @test findmin([0.0, -0.0]) === (-0.0, 2)
@@ -1814,6 +1814,11 @@ s, si = findmax(S)
 @test a == b == s
 @test ai == bi == si
 
+for X in (A, B, S)
+    @test findmin(X) == findmin(Dict(pairs(X)))
+    @test findmax(X) == findmax(Dict(pairs(X)))
+end
+
 fill!(B, 2)
 @test all(x->x==2, B)