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

Extend LinearAlgebra.copy_oftype #134

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Extend LinearAlgebra.copy_oftype #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17aeb65
Add erroring test
mjp98 Nov 16, 2022
bc6e152
Extend `LinearAlgebra.copy_oftype`
mjp98 Nov 16, 2022
fed0bcd
Bump patch version
mjp98 Nov 16, 2022
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
2 changes: 1 addition & 1 deletion Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "AxisKeys"
uuid = "94b1ba4f-4ee9-5380-92f1-94cde586c3c5"
license = "MIT"
version = "0.2.10"
version = "0.2.11"

[deps]
AbstractFFTs = "621f4979-c628-5d54-868e-fcf4e3e8185c"
Expand Down
6 changes: 5 additions & 1 deletion src/functions.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,11 @@ LinearAlgebra.cholesky(A::Hermitian{T, <:KeyedArray{T}}; kwargs...) where {T} =
LinearAlgebra.cholesky(A::KeyedMatrix; kwargs...) =
cholesky(keyless_unname(A); kwargs...)

# Defer to LinearAlgebra.copy_oftype for parent Array - see issue #133
function LinearAlgebra.copy_oftype(A::KeyedArray, ::Type{T}) where {T}
return KeyedArray(LinearAlgebra.copy_oftype(parent(A), T), axiskeys(A))
end

function Base.deleteat!(v::KeyedVector, inds)
deleteat!(axiskeys(v, 1), inds)
deleteat!(v.data, inds)
Expand All @@ -441,4 +446,3 @@ function Base.filter!(f, a::KeyedVector)
deleteat!(a, j:lastindex(a))
return a
end

6 changes: 5 additions & 1 deletion test/_functions.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ end

@test axiskeys(filter(isodd, V2),1) isa Vector{Int}
@test dimnames(filter(isodd, V2)) == (:v,)

V4 = wrapdims(rand(1:99, 10), collect(2:11))
V4c = copy(V4)
@test filter!(isodd, V4c) === V4c == filter(isodd, V4)
Expand Down Expand Up @@ -352,6 +352,10 @@ end
@test axiskeys(T10 \ V, 1) == 1:3
end

# Catch issue #133
S = wrapdims(Symmetric(rand(Int8,3,3)), 'a':'c', 10:10:30)
@test LinearAlgebra.copy_oftype(S, eltype(S)) == S
Comment on lines +355 to +357
Copy link
Owner

Choose a reason for hiding this comment

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

This test passes before the PR.

It's OK to test the exact internal function, but more important I think to test the high-level behaviour which needs it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I will add a test of the high level problem.

Sorry, I think I misdiagnosed the problem, which seems to be with copyto! (which my Julia version was calling) This errors on Julia 1.6

julia> versioninfo()
Julia Version 1.6.7
Commit 3b76b25b64 (2022-07-19 15:11 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin21.4.0)
  CPU: Apple M1 Pro
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-11.0.1 (ORCJIT, westmere)

julia> S =  wrapdims(Symmetric(rand(Int8,3,3)), 'a':'c', 10:10:30)
2-dimensional KeyedArray(...) with keys:
↓   3-element StepRange{Char,...}
→   3-element StepRange{Int64,...}
And data, 3×3 Symmetric{Int8, Matrix{Int8}}:
         (10)  (20)  (30)
  ('a')   -37  -122    -2
  ('b')  -122    90  -117
  ('c')    -2  -117   -12

julia> A = copy(S)
2-dimensional KeyedArray(...) with keys:
↓   3-element StepRange{Char,...}
→   3-element StepRange{Int64,...}
And data, 3×3 Symmetric{Int8, Matrix{Int8}}:
         (10)  (20)  (30)
  ('a')   -37  -122    -2
  ('b')  -122    90  -117
  ('c')    -2  -117   -12

julia> copyto!(A, S)
ERROR: ArgumentError: Cannot set a non-diagonal index in a symmetric matrix
Stacktrace:
 [1] setindex!
   @ /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.6/LinearAlgebra/src/symmetric.jl:225 [inlined]
 [2] setindex!
   @ ~/.julia/packages/AxisKeys/DiXQB/src/struct.jl:132 [inlined]
 [3] copyto_unaliased!(deststyle::IndexCartesian, dest::KeyedArray{Int8, 2, Symmetric{Int8, Matrix{Int8}}, Tuple{StepRange{Char, Int64}, StepRange{Int64, Int64}}}, srcstyle::IndexCartesian, src::KeyedArray{Int8, 2, Symmetric{Int8, Matrix{Int8}}, Tuple{StepRange{Char, Int64}, StepRange{Int64, Int64}}})
   @ Base ./abstractarray.jl:984
 [4] copyto!(dest::KeyedArray{Int8, 2, Symmetric{Int8, Matrix{Int8}}, Tuple{StepRange{Char, Int64}, StepRange{Int64, Int64}}}, src::KeyedArray{Int8, 2, Symmetric{Int8, Matrix{Int8}}, Tuple{StepRange{Char, Int64}, StepRange{Int64, Int64}}})
   @ Base ./abstractarray.jl:950
 [5] top-level scope
   @ REPL[14]:1

julia> @which copyto!(A, S)
copyto!(dest::AbstractArray, src::AbstractArray) in Base at abstractarray.jl:947


@testset "cholesky" begin
A = rand(10, 3)
ka = KeyedArray(A, (0:9, ["a", "b", "c"]))
Expand Down