Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop support for non-indexable iterators in find* functions #26095

Merged
merged 2 commits into from
Feb 19, 2018
Merged

Drop support for non-indexable iterators in find* functions #26095

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
29831a1
Drop support for non-indexable iterators in find* functions
nalimilan Feb 17, 2018
31aa07d
Fix "after/before or including i"
nalimilan Feb 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 17 additions & 36 deletions base/array.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1497,17 +1497,10 @@ 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 `i` of a `true` element of `A`, or `nothing` if not found.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After or including i.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.


Indices are of the same type as those returned by [`keys(A)`](@ref)
and [`pairs(A)`](@ref).
Expand Down Expand Up @@ -1562,9 +1555,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 +1583,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,7 +1601,7 @@ 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`,
Find the next index after `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)
Expand Down Expand Up @@ -1659,9 +1650,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 +1680,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 +1693,7 @@ 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 `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 +1744,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 +1774,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,7 +1792,7 @@ 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
Find the previous index before `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)
Expand Down Expand Up @@ -1851,9 +1838,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 +1865,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 +1882,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 +1926,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 +1936,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 +1970,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
Open in desktop
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