From bd772d78883930ad27853f17be7b4a5628fd5612 Mon Sep 17 00:00:00 2001 From: Milan Bouchet-Valat Date: Tue, 16 Jan 2018 10:13:14 +0100 Subject: [PATCH] Use more generic approach for findfirst and findlast Also add tests, and remove @inferred calls which did not actually check whether functions were inferred, but only whether the equality test was. Change the _pairs() fallback to use Iterators.countfrom() so that an error is raised by findlast() rather than assuming the last index is typemax(Int), and use length() for iterators which support it so that findlast() works. --- base/array.jl | 102 +++++++++++++++++++++++++++++++++++---------- test/arrayops.jl | 35 +++++++++++++++- test/dict.jl | 13 ++++-- test/namedtuple.jl | 16 +++++-- test/tuple.jl | 24 +++++++++-- 5 files changed, 155 insertions(+), 35 deletions(-) diff --git a/base/array.jl b/base/array.jl index 3a0d3637f367b2..a03bf2378f5458 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1492,6 +1492,11 @@ cat(n::Integer, x::Integer...) = reshape([x...], (ntuple(x->1, n-1)..., length(x ## find ## +_pairs(A::Union{AbstractArray, AbstractDict, AbstractString, Tuple, NamedTuple}) = pairs(A) +_pairs(iter) = _pairs(IteratorSize(iter), iter) +_pairs(::Union{HasLength, HasShape}, iter) = zip(1:length(iter), iter) +_pairs(::Union{SizeUnknown, IsInfinite}, iter) = zip(Iterators.countfrom(1), iter) + """ findnext(A, i::Integer) @@ -1545,12 +1550,14 @@ end """ findfirst(A) -Return the index of the first `true` value in `A`. +Return the index or key of the first `true` value in `A`. Return `nothing` if no such value is found. To search for other kinds of values, pass a predicate as the first argument. -Indices are of the same type as those returned by [`keys(A)`](@ref) -and [`pairs(A)`](@ref). +Indices or keys are of the same type as those returned by [`keys(A)`](@ref) +and [`pairs(A)`](@ref) for `AbstractArray`, `AbstractDict`, `AbstractString` +`Tuple` and `NamedTuple` objects, and are linear indices starting at `1` +for other iterables. # Examples ```jldoctest @@ -1576,7 +1583,22 @@ julia> findfirst(A) CartesianIndex(2, 1) ``` """ -findfirst(A) = isempty(A) ? nothing : findnext(A, first(keys(A))) +function findfirst(A) + warned = false + for (i, a) in _pairs(A) + if !warned && !(a isa Bool) + depwarn("In the future `findfirst` will only work on boolean collections. Use `findfirst(x->x!=0, A)` instead.", :findfirst) + warned = true + end + if a != 0 + return i + end + end + return nothing +end + +# Needed for bootstrap, and allows defining only an optimized findnext method +findfirst(A::Union{AbstractArray, AbstractString}) = findnext(A, first(keys(A))) """ findnext(predicate::Function, A, i) @@ -1626,11 +1648,13 @@ end """ findfirst(predicate::Function, A) -Return the index of the first element of `A` for which `predicate` returns `true`. +Return the index or key of the first element of `A` for which `predicate` returns `true`. Return `nothing` if there is no such element. -Indices are of the same type as those returned by [`keys(A)`](@ref) -and [`pairs(A)`](@ref). +Indices or keys are of the same type as those returned by [`keys(A)`](@ref) +and [`pairs(A)`](@ref) for `AbstractArray`, `AbstractDict`, `AbstractString` +`Tuple` and `NamedTuple` objects, and are linear indices starting at `1` +for other iterables. # Examples ```jldoctest @@ -1659,7 +1683,16 @@ julia> findfirst(iseven, A) CartesianIndex(2, 1) ``` """ -findfirst(testf::Function, A) = isempty(A) ? nothing : findnext(testf, A, first(keys(A))) +function findfirst(testf::Function, A) + for (i, a) in _pairs(A) + testf(a) && return i + end + return nothing +end + +# Needed for bootstrap, and allows defining only an optimized findnext method +findfirst(testf::Function, A::Union{AbstractArray, AbstractString}) = + findnext(testf, A, first(keys(A))) """ findprev(A, i) @@ -1711,11 +1744,13 @@ end """ findlast(A) -Return the index of the last `true` value in `A`. +Return the index or key of the last `true` value in `A`. Return `nothing` if there is no `true` value in `A`. -Indices are of the same type as those returned by [`keys(A)`](@ref) -and [`pairs(A)`](@ref). +Indices or keys are of the same type as those returned by [`keys(A)`](@ref) +and [`pairs(A)`](@ref) for `AbstractArray`, `AbstractDict`, `AbstractString` +`Tuple` and `NamedTuple` objects, and are linear indices starting at `1` +for other iterables. # Examples ```jldoctest @@ -1743,7 +1778,22 @@ julia> findlast(A) CartesianIndex(2, 1) ``` """ -findlast(A) = isempty(A) ? nothing : findprev(A, last(keys(A))) +function findlast(A) + warned = false + for (i, a) in Iterators.reverse(_pairs(A)) + if !warned && !(a isa Bool) + depwarn("In the future `findlast` will only work on boolean collections. Use `findlast(x->x!=0, A)` instead.", :findlast) + warned = true + end + if a != 0 + return i + end + end + return nothing +end + +# Needed for bootstrap, and allows defining only an optimized findprev method +findlast(A::Union{AbstractArray, AbstractString}) = findprev(A, last(keys(A))) """ findprev(predicate::Function, A, i) @@ -1790,11 +1840,13 @@ end """ findlast(predicate::Function, A) -Return the index of the last element of `A` for which `predicate` returns `true`. +Return the index or key of the last element of `A` for which `predicate` returns `true`. Return `nothing` if there is no such element. -Indices are of the same type as those returned by [`keys(A)`](@ref) -and [`pairs(A)`](@ref). +Indices or keys are of the same type as those returned by [`keys(A)`](@ref) +and [`pairs(A)`](@ref) for `AbstractArray`, `AbstractDict`, `AbstractString` +`Tuple` and `NamedTuple` objects, and are linear indices starting at `1` +for other iterables. # Examples ```jldoctest @@ -1820,7 +1872,16 @@ julia> findlast(isodd, A) CartesianIndex(2, 1) ``` """ -findlast(testf::Function, A) = isempty(A) ? nothing : findprev(testf, A, last(keys(A))) +function findlast(testf::Function, A) + for (i, a) in Iterators.reverse(_pairs(A)) + testf(a) && return i + end + return nothing +end + +# Needed for bootstrap, and allows defining only an optimized findprev method +findlast(testf::Function, A::Union{AbstractArray, AbstractString}) = + findprev(testf, A, last(keys(A))) """ find(f::Function, A) @@ -1828,8 +1889,10 @@ findlast(testf::Function, A) = isempty(A) ? nothing : findprev(testf, A, last(ke Return a vector `I` of the indices or keys of `A` where `f(A[I])` returns `true`. If there are no such elements of `A`, return an empty array. -Indices are of the same type as those returned by [`keys(A)`](@ref) -and [`pairs(A)`](@ref). +Indices or keys are of the same type as those returned by [`keys(A)`](@ref) +and [`pairs(A)`](@ref) for `AbstractArray`, `AbstractDict`, `AbstractString` +`Tuple` and `NamedTuple` objects, and are linear indices starting at `1` +for other iterables. # Examples ```jldoctest @@ -1875,9 +1938,6 @@ julia> find(x -> x >= 0, d) """ find(testf::Function, A) = collect(first(p) for p in _pairs(A) if testf(last(p))) -_pairs(A::Union{AbstractArray, AbstractDict, AbstractString, Tuple, NamedTuple}) = pairs(A) -_pairs(iter) = zip(OneTo(typemax(Int)), iter) # safe for objects that don't implement length - """ find(A) diff --git a/test/arrayops.jl b/test/arrayops.jl index 6dac579200b765..cea76fe1efe351 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -465,13 +465,44 @@ end @test find(isodd, A) == [CartesianIndex(1, 1), CartesianIndex(2, 1)] @test find(!iszero, A) == [CartesianIndex(1, 1), CartesianIndex(2, 1), CartesianIndex(1, 2), CartesianIndex(2, 2)] + @test findfirst(isodd, A) == CartesianIndex(1, 1) + @test findlast(isodd, A) == CartesianIndex(2, 1) + @test findnext(isodd, A, CartesianIndex(1, 1)) == CartesianIndex(1, 1) + @test findprev(isodd, A, CartesianIndex(2, 1)) == CartesianIndex(2, 1) + @test findnext(isodd, A, CartesianIndex(1, 2)) === nothing + @test findprev(iseven, A, CartesianIndex(2, 1)) === nothing end @testset "find with general iterables" begin s = "julia" @test find(c -> c == 'l', s) == [3] + @test findfirst(c -> c == 'l', s) == 3 + @test findlast(c -> c == 'l', s) == 3 + @test findnext(c -> c == 'l', s, 1) == 3 + @test findprev(c -> c == 'l', s, 5) == 3 + @test findnext(c -> c == 'l', s, 4) === nothing + @test findprev(c -> c == 'l', s, 2) === nothing + g = Base.Unicode.graphemes("日本語") - @test find(isascii, g) == Int[] - @test find(!iszero, (i % 2 for i in 1:10)) == 1:2:9 + @test find(!isempty, g) == 1:3 + @test isempty(find(isascii, g)) + @test findfirst(!isempty, g) == 1 + @test findfirst(isascii, g) === nothing + # Check that the last index isn't assumed to be typemax(Int) + @test_throws MethodError findlast(!iszero, g) + + g2 = (i % 2 for i in 1:10) + @test find(!iszero, g2) == 1:2:9 + @test findfirst(!iszero, g2) == 1 + @test findlast(!iszero, g2) == 9 + @test findfirst(equalto(2), g2) === nothing + @test findlast(equalto(2), g2) === nothing + + g3 = (i % 2 for i in 1:10, j in 1:2) + @test find(!iszero, g3) == 1:2:19 + @test findfirst(!iszero, g3) == 1 + @test findlast(!iszero, g3) == 19 + @test findfirst(equalto(2), g3) === nothing + @test findlast(equalto(2), g3) === nothing end @testset "findmin findmax indmin indmax" begin diff --git a/test/dict.jl b/test/dict.jl index 8b3cd568c1dd6a..1f65c5d5a7aa3e 100644 --- a/test/dict.jl +++ b/test/dict.jl @@ -761,8 +761,13 @@ end end @testset "find" begin - @test @inferred find(equalto(1), Dict(:a=>1, :b=>2)) == [:a] - @test @inferred sort(find(equalto(1), Dict(:a=>1, :b=>1))) == [:a, :b] - @test @inferred isempty(find(equalto(1), Dict())) - @test @inferred isempty(find(equalto(1), Dict(:a=>2, :b=>3))) + @test find(equalto(1), Dict(:a=>1, :b=>2)) == [:a] + @test sort(find(equalto(1), Dict(:a=>1, :b=>1))) == [:a, :b] + @test isempty(find(equalto(1), Dict())) + @test isempty(find(equalto(1), Dict(:a=>2, :b=>3))) + + @test findfirst(equalto(1), Dict(:a=>1, :b=>2)) == :a + @test findfirst(equalto(1), Dict(:a=>1, :b=>1, :c=>3)) in (:a, :b) + @test findfirst(equalto(1), Dict()) === nothing + @test findfirst(equalto(1), Dict(:a=>2, :b=>3)) === nothing end \ No newline at end of file diff --git a/test/namedtuple.jl b/test/namedtuple.jl index df5e5d274e3f33..555441be095374 100644 --- a/test/namedtuple.jl +++ b/test/namedtuple.jl @@ -209,7 +209,15 @@ abstr_nt_22194_3() @test typeof(Base.structdiff(NamedTuple{(:a, :b), Tuple{Int32, Union{Int32, Nothing}}}((1, Int32(2))), (a=0,))) === NamedTuple{(:b,), Tuple{Union{Int32, Nothing}}} -@test @inferred find(equalto(1), (a=1, b=2)) == [:a] -@test @inferred find(equalto(1), (a=1, b=1)) == [:a, :b] -@test @inferred isempty(find(equalto(1), NamedTuple())) -@test @inferred isempty(find(equalto(1), (a=2, b=3))) \ No newline at end of file +@test find(equalto(1), (a=1, b=2)) == [:a] +@test find(equalto(1), (a=1, b=1)) == [:a, :b] +@test isempty(find(equalto(1), NamedTuple())) +@test isempty(find(equalto(1), (a=2, b=3))) +@test findfirst(equalto(1), (a=1, b=2)) == :a +@test findlast(equalto(1), (a=1, b=2)) == :a +@test findfirst(equalto(1), (a=1, b=1)) == :a +@test findlast(equalto(1), (a=1, b=1)) == :b +@test findfirst(equalto(1), ()) === nothing +@test findlast(equalto(1), ()) === nothing +@test findfirst(equalto(1), (a=2, b=3)) === nothing +@test findlast(equalto(1), (a=2, b=3)) === nothing \ No newline at end of file diff --git a/test/tuple.jl b/test/tuple.jl index f7f5d0260da310..a744f291696261 100644 --- a/test/tuple.jl +++ b/test/tuple.jl @@ -366,8 +366,24 @@ end end @testset "find" begin - @test @inferred find(equalto(1), (1, 2)) == [1] - @test @inferred find(equalto(1), (1, 1)) == [1, 2] - @test @inferred isempty(find(equalto(1), ())) - @test @inferred isempty(find(equalto(1), (2, 3))) + @test find(equalto(1), (1, 2)) == [1] + @test find(equalto(1), (1, 1)) == [1, 2] + @test isempty(find(equalto(1), ())) + @test isempty(find(equalto(1), (2, 3))) + + @test findfirst(equalto(1), (1, 2)) == 1 + @test findlast(equalto(1), (1, 2)) == 1 + @test findfirst(equalto(1), (1, 1)) == 1 + @test findlast(equalto(1), (1, 1)) == 2 + @test findfirst(equalto(1), ()) === nothing + @test findlast(equalto(1), ()) === nothing + @test findfirst(equalto(1), (2, 3)) === nothing + @test findlast(equalto(1), (2, 3)) === nothing + + @test findnext(equalto(1), (1, 2), 1) == 1 + @test findprev(equalto(1), (1, 2), 2) == 1 + @test findnext(equalto(1), (1, 1), 2) == 2 + @test findprev(equalto(1), (1, 1), 1) == 1 + @test findnext(equalto(1), (2, 3), 1) === nothing + @test findprev(equalto(1), (2, 3), 2) === nothing end \ No newline at end of file