Skip to content

Commit

Permalink
RFC: Neither advertise full keyword of qr nor test it on 0.7 (#558)
Browse files Browse the repository at this point in the history
* Neither advertise `full` keyword of `qr` nor test it on 0.7

The `thin` keyword was replaced by `full`, but later abandoned
completely. To keep code working on Julia 0.6 that had adapted to
`full`, the interface is retained on Julia 0.6, but neither tested on
0.7 (where it doesn't work) nor advertised in the README.

* Add brief comments explaining the qr situation
  • Loading branch information
martinholters committed May 29, 2018
1 parent 0f612e1 commit cdde087
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 21 deletions.
4 changes: 1 addition & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ Currently, the `@compat` macro supports the following syntaxes:

* `fetch` for `Task`s ([#25940]).

* `Compat.qr` takes `pivot` as a `Val` _instance_ and keyword argument `full` ([#22475], [#24279]).
* `Compat.qr` takes `pivot` as a `Val` _instance_ ([#22475]).

* `Compat.rmul!` provides a subset of the functionality of `LinearAlgebra.rmul!` for
use with Julia 0.6 ([#25701], [#25812]).
Expand Down Expand Up @@ -511,7 +511,6 @@ includes this fix. Find the minimum version from there.
[#18510]: https://github.com/JuliaLang/julia/issues/18510
[#18629]: https://github.com/JuliaLang/julia/issues/18629
[#18727]: https://github.com/JuliaLang/julia/issues/18727
[#18839]: https://github.com/JuliaLang/julia/issues/18839
[#18977]: https://github.com/JuliaLang/julia/issues/18977
[#19088]: https://github.com/JuliaLang/julia/issues/19088
[#19246]: https://github.com/JuliaLang/julia/issues/19246
Expand Down Expand Up @@ -561,7 +560,6 @@ includes this fix. Find the minimum version from there.
[#23931]: https://github.com/JuliaLang/julia/issues/23931
[#24047]: https://github.com/JuliaLang/julia/issues/24047
[#24182]: https://github.com/JuliaLang/julia/issues/24182
[#24279]: https://github.com/JuliaLang/julia/issues/24279
[#24282]: https://github.com/JuliaLang/julia/issues/24282
[#24361]: https://github.com/JuliaLang/julia/issues/24361
[#24372]: https://github.com/JuliaLang/julia/issues/24372
Expand Down
2 changes: 2 additions & 0 deletions src/Compat.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1818,6 +1818,8 @@ if !isdefined(Base, :selectdim) # 0.7.0-DEV.3976
end

if VERSION < v"0.7.0-DEV.2337"
# qr doesn't take the full keyword anymore since 0.7.0-DEV.5211; we still support it
# here to avoid unneccesary breakage
if VERSION < v"0.7.0-DEV.843"
qr(A::Union{Number,AbstractMatrix}, pivot::Union{Val{false},Val{true}}=Val(false); full=false) =
Base.qr(A, typeof(pivot), thin=!full)
Expand Down
48 changes: 30 additions & 18 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1674,24 +1674,36 @@ end

# 0.7.0-DEV.843 / 0.7.0-DEV.2337
let A = [1 2; 1 2; 1 2]
f = Compat.qr(A, Val(false), full=false)
@test f == Compat.qr(A, Val(false))
@test length(f) == 2
@test size(f[1]) == (3, 2)
@test f[1] * f[2] A
f = Compat.qr(A, Val(false), full=true)
@test length(f) == 2
@test size(f[1]) == (3, 3)
@test f[1] * [f[2]; [0 0]] A
f = Compat.qr(A, Val(true), full=false)
@test f == Compat.qr(A, Val(true))
@test length(f) == 3
@test size(f[1]) == (3, 2)
@test f[1] * f[2] A[:,f[3]]
f = Compat.qr(A, Val(true), full=true)
@test length(f) == 3
@test size(f[1]) == (3, 3)
@test f[1] * [f[2]; [0 0]] A[:,f[3]]
if VERSION < v"0.7.0-DEV.5211"
# the full keyword was only temporarily available in Base, so these methods don't
# work on 0.7 anymore, but we test them for the time being to avoid accidentally
# breaking anyone's code
f = Compat.qr(A, Val(false), full=false)
@test f == Compat.qr(A, Val(false))
@test length(f) == 2
@test size(f[1]) == (3, 2)
@test f[1] * f[2] A
f = Compat.qr(A, Val(false), full=true)
@test length(f) == 2
@test size(f[1]) == (3, 3)
@test f[1] * [f[2]; [0 0]] A
f = Compat.qr(A, Val(true), full=false)
@test f == Compat.qr(A, Val(true))
@test length(f) == 3
@test size(f[1]) == (3, 2)
@test f[1] * f[2] A[:,f[3]]
f = Compat.qr(A, Val(true), full=true)
@test length(f) == 3
@test size(f[1]) == (3, 3)
@test f[1] * [f[2]; [0 0]] A[:,f[3]]
else
f = Compat.qr(A, Val(false))
@test size(f.Q) == (3, 3)
@test f.Q * [f.R; [0 0]] A
f = Compat.qr(A, Val(true))
@test size(f.Q) == (3, 3)
@test f.Q * [f.R; [0 0]] A[:,f.p]
end
end

let A = [1 2; 3 4]
Expand Down

0 comments on commit cdde087

Please sign in to comment.