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

Change find() to return the same index type as pairs() #24774

Merged
merged 4 commits into from
Jan 10, 2018
Merged

Change find() to return the same index type as pairs() #24774

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a16323e
Change find() to return the same index type as pairs()
nalimilan Nov 25, 2017
dee7afd
Add NEWS entry
nalimilan Jan 6, 2018
b286f17
Also return indices from keys() for AbstractString, Tuple and NamedTu…
nalimilan Jan 7, 2018
8e605fb
Merge branch 'master' into nl/find
JeffBezanson Jan 9, 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
Prev Previous commit
Next Next commit
Also return indices from keys() for AbstractString, Tuple and NamedTu… 
…ple, and add tests
  • Loading branch information
@nalimilan
nalimilan committed Jan 7, 2018
commit b286f17b0b99935a199a60b432d77b7525f23914
10 changes: 5 additions & 5 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,11 @@ This section lists changes that do not have deprecation warnings.
* `findn(x::AbstractVector)` now returns a 1-tuple with the vector of indices, to be
consistent with higher order arrays ([#25365]).

* `find` now returns the same type of indices as `keys`/`pairs` for `AbstractArray` and
`AbstractDict` objects ([#24774]). In particular, this means that it returns
`CartesianIndex` objects for matrices and higher-dimensional arrays instead of
always returning linear indices as it was previously the case. Use
`Int[LinearIndices(size(a))[i] for i in find(f, a)]` to compute linear indices
* `find` now returns the same type of indices as `keys`/`pairs` for `AbstractArray`,
`AbstractDict`, `AbstractString`, `Tuple` and `NamedTuple` objects ([#24774]).
In particular, this means that it returns `CartesianIndex` objects for matrices
and higher-dimensional arrays instead of linear indices as it was previously the case.
Use `Int[LinearIndices(size(a))[i] for i in find(f, a)]` to compute linear indices.

Library improvements
--------------------
Expand Down
7 changes: 4 additions & 3 deletions base/array.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1724,8 +1724,9 @@ 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` and `Associative` objects,
and are linear indices of type `Int` for other iterables.
and [`pairs(A)`](@ref) for `AbstractArray`, `AbstractDict`, `AbstractString`
`Tuple` and `NamedTuple` objects, and are linear indices starting at `1`
for other iterables.

# Examples
```jldoctest
Expand Down Expand Up @@ -1771,7 +1772,7 @@ 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}) = pairs(A)
_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
Copy link
Member

Choose a reason for hiding this comment

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

Can't you check the iterator trait here?

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean? We don't guarantee that iterators implement pairs currently.

Copy link
Member

Choose a reason for hiding this comment

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

This should probably use countfrom(1).


"""
Expand Down
4 changes: 0 additions & 4 deletions test/arrayops.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,10 +471,6 @@ end
@test find(!iszero, A) == [CartesianIndex(1, 1), CartesianIndex(2, 1),
CartesianIndex(1, 2), CartesianIndex(2, 2)]
end
@testset "find with Dict" begin
d = Dict(:A => 10, :B => -1, :C => 0)
@test sort(find(x -> x >= 0, d)) == [:A, :C]
end
@testset "find with general iterables" begin
s = "julia"
@test find(c -> c == 'l', s) == [3]
Expand Down
7 changes: 7 additions & 0 deletions test/dict.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -757,3 +757,10 @@ end
end
@test map(string, keys(d)) == Set(["1","3"])
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)))
end
5 changes: 5 additions & 0 deletions test/namedtuple.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,3 +208,8 @@ abstr_nt_22194_3()
@test Base.structdiff((a=1, b=2, z=20), NamedTuple{(:b,)}) == (a=1, z=20)
@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)))
4 changes: 4 additions & 0 deletions test/strings/search.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,3 +324,7 @@ end
@test findnext(equalto('('), "(⨳(", 2) == 5
@test findlast(equalto('('), "(⨳(") == 5
@test findprev(equalto('('), "(⨳(", 2) == 1

@test @inferred find(equalto('a'), "éa") == [3]
@test @inferred find(equalto('€'), "€€") == [1, 4]
@test @inferred isempty(find(equalto('é'), ""))
7 changes: 7 additions & 0 deletions test/tuple.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,3 +364,10 @@ end
@testset "issue 24707" begin
@test eltype(Tuple{Vararg{T}} where T<:Integer) >: Integer
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)))
end