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

Fix find() when called with function and non-StridedArray #8323

Merged
merged 1 commit into from
Sep 16, 2014
Merged

Fix find() when called with function and non-StridedArray #8323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions base/array.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ function findnext(testf::Function, A, start::Integer)
end
findfirst(testf::Function, A) = findnext(testf, A, 1)

function find(testf::Function, A::StridedArray)
function find(testf::Function, A::AbstractArray)
# use a dynamic-length array to store the indexes, then copy to a non-padded
# array for the return
tmpI = Array(Int, 0)
Expand All @@ -1063,7 +1063,7 @@ function find(testf::Function, A::StridedArray)
push!(tmpI, i)
end
end
I = similar(A, Int, length(tmpI))
I = Array(Int, length(tmpI))
copy!(I, tmpI)
I
end
Expand All @@ -1082,7 +1082,7 @@ function find(A::StridedArray)
end

find(x::Number) = x == 0 ? Array(Int,0) : [1]
find(testf::Function, x) = find(testf(x))
find(testf::Function, x::Number) = testf(x) == 0 ? Array(Int,0) : [1]
Copy link
Member

Choose a reason for hiding this comment

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

Should be !testf(x) instead of testf(x)==0, but false == 0 so this works.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, not pretty. :-/ I could have fixed that. Worth another commit?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure, but a commit is cheap: 6b3276f

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but for me it implies bothering somebody with a pull request.

While we're tweaking style, how about writing it testf(x) ? [1] : Array(Int,0) instead? :-p

Copy link
Member

Choose a reason for hiding this comment

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

Or

testf(x) ? Int[1] : Int[]

Which will also make the function type stable. (Sorry! Array constructors confuse me)

I don't think Pull requests is bothering for small changes like this, that are obviously correct. A unrelated discussion that goes on and on, is a different matter tough.

Copy link
Member

Choose a reason for hiding this comment

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

Or testf(x) ? [1] : Int[] ?!


findn(A::AbstractVector) = find(A)

Expand Down
4 changes: 4 additions & 0 deletions test/sparse.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ mfe22 = eye(Float64, 2)
K,J,V = findnz(SparseMatrixCSC(2,1,[1,3],[1,2],[1.0,0.0]))
@test length(K) == length(J) == length(V) == 1

# https://groups.google.com/d/msg/julia-users/Yq4dh8NOWBQ/GU57L90FZ3EJ
A = speye(Bool, 5)
@test find(A) == find(x -> x == true, A) == find(full(A))

# issue #5437
@test nnz(sparse([1,2,3],[1,2,3],[0.0,1.0,2.0])) == 2

Expand Down