davidavdav / NamedArrays.jl

Julia type that implements a drop-in replacement of Array with named dimensions
Other
118 stars 20 forks source link

Incorrect permutedims #117

Closed bkamins closed 1 year ago

bkamins commented 2 years ago

Example:

julia> using FreqTables

julia> using CategoricalArrays

julia> x = categorical(["a", missing, "b", "b"])
4-element CategoricalArray{Union{Missing, String},1,UInt32}:
 "a"
 missing
 "b"
 "b"

julia> y = categorical(["a", missing, "c", "d"])
4-element CategoricalArray{Union{Missing, String},1,UInt32}:
 "a"
 missing
 "c"
 "d"

julia> f = freqtable(x, y)
3×4 Named Matrix{Int64}
Dim1 ╲ Dim2 │     "a"      "c"      "d"  missing
────────────┼───────────────────────────────────
"a"         │       1        0        0        0
"b"         │       0        1        1        0
missing     │       0        0        0        1

julia> permutedims(f)
ERROR: MethodError: Cannot `convert` an object of type String to an object of type CategoricalValue{String, UInt32}

The reason is we need a set of custom permutedims methods for NamedArray type. E.g. this is one such custom method (not in full generality but works):

permutedims(a::NamedMatrix) = NamedArray(permutedims(a.array), reverse(a.dicts), reverse(a.dimnames))
bkamins commented 2 years ago

Another example of wrong behavior:

julia> x = categorical(["a", missing, "b"])
3-element CategoricalArray{Union{Missing, String},1,UInt32}:
 "a"
 missing
 "b"

julia> f = freqtable(x, 1:3)
3×3 Named Matrix{Int64}
Dim1 ╲ Dim2 │ 1  2  3
────────────┼────────
"a"         │ 1  0  0
"b"         │ 0  0  1
missing     │ 0  1  0

julia> permutedims(f)
3×3 Named Matrix{Int64}
Dim1 ╲ Dim2 │ 1  2  3
────────────┼────────
"a"         │ 1  0  0
"b"         │ 0  0  1
missing     │ 0  1  0
mitchphillipson commented 1 year ago

I've just run into a similar issue with permutedims failing.

Here is an example of the code I’m using

using NamedArrays

A = NamedArray([1 2 3;4 5 6],([:a,:b],[:c,:d,:e]))
permutedims(A,(2,1))

This fails because the NamedArrays permutedims method expects a permutation as a vector, whereas general permutedims doesn't seem to care (hence my usage of a tuple)

This code does work:

using NamedArrays

A = NamedArray([1 2 3;4 5 6],([:a,:b],[:c,:d,:e]))
permutedims(A, [2,1] ) #Permutation should be a vector

Interestingly, this issue only occurs (for me) if the names are symbols are the dimensions don't match. For example, both of these examples work without issue

using NamedArrays

A = NamedArray([1 2 3;4 5 6],(["a","b"],["c","d","e"]))
permutedims(A,(2,1))

B = NamedArray([1 2;4 5],([:a,:b],[:c,:d]))
permutedims(B,(2,1))

A "simple" fix would be to add the following function to rearrange.jl.

function permutedims(a::NamedArray, perm::Tuple{Vararg{Int}})  #could have type of perm wrong, but I believe it's correct
    permutedims(a, collect(perm))
end
davidavdav commented 1 year ago

Thanks, This should all be fixed in v0.9.8