Deprecate ordschur() methods two methods where the individual components are passed #28155
Conversation
|
|
||
| function ordschur!(T::StridedMatrix{Ty}, Z::StridedMatrix{Ty}, select::Union{Vector{Bool},BitVector}) where {Ty<:BlasFloat} | ||
| depwarn(string("`ordschur!(T::StridedMatrix{Ty}, Z::StridedMatrix{Ty}, select::Union{Vector{Bool},BitVector})`", | ||
| "`where {Ty<:BlasFloat}` is deprecated, use `ordschur!(schur::Schur, select::Union{Vector{Bool},BitVector})`", "instead.", :ordschur!) |
There was a problem hiding this comment.
There is a closing parenthesis missing here
|
Fixed the missing closing parentheses and added a NEWS.md item. Deprecation warning still does not show locally. |
|
I do see the deprecations. Do you need to adjust some of the tests as well? |
|
Sorry for the delay here. I checked https://github.com/JuliaLang/julia/blob/master/stdlib/LinearAlgebra/test/schur.jl and all the |
JeffBezanson
added
deprecation
stdlib/LinearAlgebra/src/schur.jl
Outdated
| @@ -149,27 +149,6 @@ included or both excluded via `select`. | |||
| ordschur(schur::Schur, select::Union{Vector{Bool},BitVector}) = | |||
| Schur(ordschur(schur.T, schur.Z, select)...) | |||
There was a problem hiding this comment.
It looks like there is still a call to the deprecated signature here.
There was a problem hiding this comment.
Thanks Jeff. I think the deprecation is more involved than I initially thought. The code itself uses the ordschur method with individual components in a number of places to define the method with factorization object.
Do you think it's worth going ahead and rewriting the parts or should I close this PR? A minimum PR would then only correct the wrong documentation (see #28145)
There was a problem hiding this comment.
You could change the deprecated methods that are still needed to an internal function, _ordschur.
|
Tests have passed now (failure in circleci seems unrelated if I read it correctly). Anything else to do here? Thanks for all the help! |
…nts are passed (#28155) * Deprecate ordschur methods * Add news item * Add auxiliary _ordschur and _ordschur! functions
Adresses #28145. The idea is to deprecate the
ordschurmethods that accept the individual components instead of theschurfactorization.@andreasnoack I tried to follow the example you gave but, locally, no deprecation messages are shown. One "problem" could be that I had to install 0.7 for this PR and my setup is still a bit shaky. So any feedback is welcome.