AxisKeys.jl icon indicating copy to clipboard operation
AxisKeys.jl copied to clipboard

`push!` can leave the original array in a broken state

Open dodoplus opened this issue 4 years ago • 1 comments

This seems fine:

A = KeyedArray(rand(Int8, 2); index=["a", "b"])
push!(A, 5)

1-dimensional KeyedArray(NamedDimsArray(...)) with keys:
↓   index ∈ 3-element Vector{Any}
And data, 3-element Vector{Int8}:
  ("a")   106
  ("b")  -121
 (3)        5

but:

A

ArgumentError: lengths of key vectors must match size of array (and their axes too)

Stacktrace:
  [1] construction_check
    @ ~/.julia/packages/AxisKeys/pm7Fb/src/struct.jl:30 [inlined]
  [2] KeyedArray
    @ ~/.julia/packages/AxisKeys/pm7Fb/src/struct.jl:17 [inlined]
  [3] KeyedArray
    @ ~/.julia/packages/AxisKeys/pm7Fb/src/struct.jl:20 [inlined]
  [4] unname
    @ ~/.julia/packages/AxisKeys/pm7Fb/src/names.jl:29 [inlined]
  [5] _summary(io::IOContext{IOBuffer}, x::KeyedArray{Int8, 1, NamedDimsArray{(:index,), Int8, 1, Vector{Int8}}, Base.RefValue{Vector{String}}})
    @ AxisKeys ~/.julia/packages/AxisKeys/pm7Fb/src/show.jl:16
  [6] summary
    @ ~/.julia/packages/AxisKeys/pm7Fb/src/show.jl:2 [inlined]
  [7] show(io::IOContext{IOBuffer}, #unused#::MIME{Symbol("text/plain")}, X::KeyedArray{Int8, 1, NamedDimsArray{(:index,), Int8, 1, Vector{Int8}}, Base.RefValue{Vector{String}}})
    @ Base ./arrayshow.jl:337
  [8] limitstringmime(mime::MIME{Symbol("text/plain")}, x::KeyedArray{Int8, 1, NamedDimsArray{(:index,), Int8, 1, Vector{Int8}}, Base.RefValue{Vector{String}}})
...

dodoplus avatar Oct 31 '21 09:10 dodoplus

That's bad, sorry. What should it do?

It can't push its guess into the existing array, since convert(String, 3) is an error. It can't save vcat(A.index, 3) into the original object, as this has the wrong type.

It could make a better guess for strings (but what?) but there will always be stranger types for which it can't make a good guess. Maybe more cases should be made errors?

julia> A = KeyedArray(rand(Int8, 2); index=["a", "b"]);

julia> push!(A, "c" => 55);  # given key has suitable type

julia> A
1-dimensional KeyedArray(NamedDimsArray(...)) with keys:
↓   index ∈ 3-element Vector{String}
And data, 3-element Vector{Int8}:
 ("a")  -25
 ("b")  -64
 ("c")   55

julia> push!(A, 3 => 55);
ERROR: MethodError: no method matching String(::Int64)

julia> C = KeyedArray(rand(Int8, 2); index='a':'b');

julia> push!(C, 55);  # it can guess the next key

julia> C  # immutable C.index has been replaced
1-dimensional KeyedArray(NamedDimsArray(...)) with keys:
↓   index ∈ 3-element StepRange{Char,...}
And data, 3-element Vector{Int8}:
 ('a')  54
 ('b')  88
 ('c')  55

mcabbott avatar Oct 31 '21 11:10 mcabbott