Skip to content

Commit

Permalink
Drop support for non-indexable iterators in find* functions (JuliaLan…
Browse files Browse the repository at this point in the history 
…g#26095)

Always call pairs(), which will throw an error for iterables which
do not define it. Removing the internal _pairs() function allows
supporting custom types which implement pairs().

Also fix/improve a few docstrings.
  • Loading branch information
@nalimilan @JeffBezanson
nalimilan authored and JeffBezanson committed Feb 19, 2018
1 parent 29f2195 commit e6c6ea7
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 83 deletions.
59 changes: 21 additions & 38 deletions base/array.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1497,17 +1497,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)
# includes HasShape{1} for consistency with keys(::AbstractVector)
_pairs(::Union{HasLength, HasShape{1}}, iter) = zip(1:length(iter), iter)
_pairs(::HasShape, iter) = zip(CartesianIndices(size(iter)), iter)
_pairs(::Union{SizeUnknown, IsInfinite}, iter) = zip(Iterators.countfrom(1), iter)

"""
findnext(A, i::Integer)
findnext(A, i)
Find the next linear index >= `i` of a `true` element of `A`, or `nothing` if not found.
Find the next index after or including `i` of a `true` element of `A`,
or `nothing` if not found.
Indices are of the same type as those returned by [`keys(A)`](@ref)
and [`pairs(A)`](@ref).
Expand Down Expand Up @@ -1562,9 +1556,7 @@ Return `nothing` if no such value is found.
To search for other kinds of values, pass a predicate as the first argument.
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`](@ref) objects, and are linear indices starting at `1`
for other iterables.
and [`pairs(A)`](@ref).
# Examples
```jldoctest
Expand Down Expand Up @@ -1592,7 +1584,7 @@ CartesianIndex(2, 1)
"""
function findfirst(A)
warned = false
for (i, a) in _pairs(A)
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
Expand All @@ -1610,8 +1602,8 @@ findfirst(A::Union{AbstractArray, AbstractString}) = findnext(A, first(keys(A)))
"""
findnext(predicate::Function, A, i)
Find the next index >= `i` of an element of `A` for which `predicate` returns `true`,
or `nothing` if not found.
Find the next index after or including `i` of an element of `A`
for which `predicate` returns `true`, or `nothing` if not found.
Indices are of the same type as those returned by [`keys(A)`](@ref)
and [`pairs(A)`](@ref).
Expand Down Expand Up @@ -1659,9 +1651,7 @@ Return the index or key of the first element of `A` for which `predicate` return
Return `nothing` if there is no such element.
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`](@ref) objects, and are linear indices starting at `1`
for other iterables.
and [`pairs(A)`](@ref).
# Examples
```jldoctest
Expand Down Expand Up @@ -1691,7 +1681,7 @@ CartesianIndex(2, 1)
```
"""
function findfirst(testf::Function, A)
for (i, a) in _pairs(A)
for (i, a) in pairs(A)
testf(a) && return i
end
return nothing
Expand All @@ -1704,7 +1694,8 @@ findfirst(testf::Function, A::Union{AbstractArray, AbstractString}) =
"""
findprev(A, i)
Find the previous index <= `i` of a `true` element of `A`, or `nothing` if not found.
Find the previous index before or including `i` of a `true` element of `A`,
or `nothing` if not found.
Indices are of the same type as those returned by [`keys(A)`](@ref)
and [`pairs(A)`](@ref).
Expand Down Expand Up @@ -1755,9 +1746,7 @@ Return the index or key of the last `true` value in `A`.
Return `nothing` if there is no `true` value in `A`.
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`](@ref) objects, and are linear indices starting at `1`
for other iterables.
and [`pairs(A)`](@ref).
# Examples
```jldoctest
Expand Down Expand Up @@ -1787,7 +1776,7 @@ CartesianIndex(2, 1)
"""
function findlast(A)
warned = false
for (i, a) in Iterators.reverse(_pairs(A))
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
Expand All @@ -1805,8 +1794,8 @@ findlast(A::Union{AbstractArray, AbstractString}) = findprev(A, last(keys(A)))
"""
findprev(predicate::Function, A, i)
Find the previous index <= `i` of an element of `A` for which `predicate` returns `true`, or
`nothing` if not found.
Find the previous index before or including `i` of an element of `A`
for which `predicate` returns `true`, or `nothing` if not found.
Indices are of the same type as those returned by [`keys(A)`](@ref)
and [`pairs(A)`](@ref).
Expand Down Expand Up @@ -1851,9 +1840,7 @@ Return the index or key of the last element of `A` for which `predicate` returns
Return `nothing` if there is no such element.
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`](@ref) objects, and are linear indices starting at `1`
for other iterables.
and [`pairs(A)`](@ref).
# Examples
```jldoctest
Expand All @@ -1880,7 +1867,7 @@ CartesianIndex(2, 1)
```
"""
function findlast(testf::Function, A)
for (i, a) in Iterators.reverse(_pairs(A))
for (i, a) in Iterators.reverse(pairs(A))
testf(a) && return i
end
return nothing
Expand All @@ -1897,9 +1884,7 @@ 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 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`](@ref) objects, and are linear indices starting at `1`
for other iterables.
and [`pairs(A)`](@ref).
# Examples
```jldoctest
Expand Down Expand Up @@ -1943,7 +1928,7 @@ julia> findall(x -> x >= 0, d)
```
"""
findall(testf::Function, A) = collect(first(p) for p in _pairs(A) if testf(last(p)))
findall(testf::Function, A) = collect(first(p) for p in pairs(A) if testf(last(p)))

"""
findall(A)
Expand All @@ -1953,9 +1938,7 @@ If there are no such elements of `A`, return an empty array.
To search for other kinds of values, pass a predicate as the first argument.
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`](@ref) objects, and are linear indices starting at `1`
for other iterables.
and [`pairs(A)`](@ref).
# Examples
```jldoctest
Expand Down Expand Up @@ -1989,7 +1972,7 @@ function findall(A)
if !(eltype(A) === Bool) && !all(x -> x isa Bool, A)
depwarn("In the future `findall(A)` will only work on boolean collections. Use `findall(x->x!=0, A)` instead.", :find)
end
collect(first(p) for p in _pairs(A) if last(p) != 0)
collect(first(p) for p in pairs(A) if last(p) != 0)
end
# Allocating result upfront is faster (possible only when collection can be iterated twice)
function findall(A::AbstractArray{Bool})
Expand Down
45 changes: 0 additions & 45 deletions test/arrayops.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,51 +475,6 @@ end
@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 findall(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 findall(!isempty, g) == 1:3
@test isempty(findall(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 findall(!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 findall(!iszero, g3) == findall(!iszero, collect(g3))
@test findfirst(!iszero, g3) == CartesianIndex(1, 1)
@test findlast(!iszero, g3) == CartesianIndex(9, 2)
@test findfirst(equalto(2), g3) === nothing
@test findlast(equalto(2), g3) === nothing

g4 = (x for x in [true, false, true, false])
@test findall(g4) == [1, 3]
@test findfirst(g4) == 1
@test findlast(g4) == 3

g5 = (x for x in [true false; true false])
@test findall(g5) == findall(collect(g5))
@test findfirst(g5) == CartesianIndex(1, 1)
@test findlast(g5) == CartesianIndex(2, 1)

@test findfirst(x for x in Bool[]) === nothing
@test findlast(x for x in Bool[]) === nothing
end

@testset "findmin findmax argmin argmax" begin
@test argmax([10,12,9,11]) == 2
Expand Down

0 comments on commit e6c6ea7

Please sign in to comment.