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.

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

deprecate sparse(s::UniformScaling, m::Integer) in favor of three-arg equivalent #24472

Merged
merged 1 commit into from
Nov 19, 2017

Conversation

Sacha0
Copy link
Member

@Sacha0 Sacha0 commented Nov 4, 2017

Per triage, this pull request deprecates sparse(s::UniformScaling, m::Integer) in favor of the three-argument equivalent sparse(s::UniformScaling, m::Integer, n::Integer). Best!

@Sacha0 Sacha0 added kind:deprecation This change introduces or involves a deprecation domain:linear algebra Linear algebra domain:arrays:sparse Sparse arrays labels Nov 4, 2017
@Sacha0 Sacha0 force-pushed the depsparsen branch 2 times, most recently from 66f4c7f to 6bd3c79 Compare November 4, 2017 18:43
@Sacha0
Copy link
Member Author

Sacha0 commented Nov 5, 2017

Ref. #24438 (comment). Best!

@Sacha0
Copy link
Member Author

Sacha0 commented Nov 5, 2017

Seems the deprecation somehow impacts Documenter?

@KristofferC
Copy link
Sponsor Member

Hmm, I'm seeing thousands of line of the type:

Error During Test at /home/julia/ci/worker/11rel-amd64/build/test/sparse/higherorderfns.jl:391
  Test threw an exception of type MethodError
  Expression: begin
    Q = broadcast(+, V, A, X)
    Q isa SparseMatrixCSC && Q == sparse(broadcast(+, fV, fA, fX))
end
  MethodError: no method matching sparse(::Array{Float64,2})
  Closest candidates are:
    sparse(!Matched::UniformScaling, !Matched::Integer) at deprecated.jl:55

@Sacha0
Copy link
Member Author

Sacha0 commented Nov 5, 2017

Ah, by nature of living in Base the deprecation might be masking the sparse methods from SparseArrays. Thanks for the additional clue! :)

@@ -1821,6 +1821,8 @@ function toc()
return t
end

@deprecate sparse(s::UniformScaling, m::Integer) sparse(s, m, m)
Copy link
Member

Choose a reason for hiding this comment

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

Probably just need an import SparseArrays.sparse 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.

Yes, something of that kind should do the trick. I went with @eval SparseArrays @deprecate ...; let's see what CI says. Thanks! :)

@Sacha0
Copy link
Member Author

Sacha0 commented Nov 18, 2017

Rebased and (hopefully) fixed the test/sparse/spqr.jl failures. Assuming CI approves and absent objections or requests for time, I plan to merge this pull request tomorrow. Best!

@Sacha0 Sacha0 merged commit 7c2842f into JuliaLang:master Nov 19, 2017
@Sacha0 Sacha0 deleted the depsparsen branch November 19, 2017 16:38
@Sacha0
Copy link
Member Author

Sacha0 commented Nov 19, 2017

Thanks all! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:arrays:sparse Sparse arrays domain:linear algebra Linear algebra kind:deprecation This change introduces or involves a deprecation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants