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.

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

setindex and promotion #131

Closed
cossio opened this issue Jun 8, 2020 · 4 comments
Closed

setindex and promotion #131

cossio opened this issue Jun 8, 2020 · 4 comments
Milestone

Comments

@cossio
Copy link

cossio commented Jun 8, 2020

julia> sa = StructArray{Complex}((randn(2), randn(2)));
julia> sa[1] = false
ERROR: type Bool has no field re
Stacktrace:
 [1] getproperty(::Bool, ::Symbol) at ./Base.jl:33
 [2] macro expansion at /home/cossio/.julia/packages/StructArrays/2PoXh/src/utils.jl:46 [inlined]
 [3] foreachfield_gen(::NamedTuple{(:re, :im),Tuple{Array{Float64,1},Array{Float64,1}}}, ::StructArrays.var"#58#59"{Int64}, ::StructArray{Complex,1,NamedTuple{(:re, :im),Tuple{Array{Float64,1},Array{Float64,1}}},Int64}, ::Bool) at /home/cossio/.julia/packages/StructArrays/2PoXh/src/utils.jl:46
 [4] foreachfield(::Function, ::StructArray{Complex,1,NamedTuple{(:re, :im),Tuple{Array{Float64,1},Array{Float64,1}}},Int64}, ::Bool) at /home/cossio/.julia/packages/StructArrays/2PoXh/src/utils.jl:51
 [5] setindex!(::StructArray{Complex,1,NamedTuple{(:re, :im),Tuple{Array{Float64,1},Array{Float64,1}}},Int64}, ::Bool, ::Int64) at /home/cossio/.julia/packages/StructArrays/2PoXh/src/structarray.jl:166
 [6] top-level scope at REPL[21]:1

Compare that with the behavior of normal arrays:

julia> X = randn(2,2) + randn(2,2) * im;
julia> X[1] = false;
julia> X
2×2 Array{Complex{Float64},2}:
      0.0+0.0im        0.872904-2.44604im
 0.300476-0.454121im  -0.156811+0.00139102im

I think StructArray should replicate the behavior of normal arrays, and promote the value being assigned.

@piever piever added this to the 1.0 milestone Jul 17, 2020
@goretkin
Copy link
Contributor

I might have missed this because it's not really about promote but more directly convert.

@fbanning
Copy link

fbanning commented Apr 21, 2021

Just stumbled upon this issue and the connected issues #182 and PR #184 because I experienced a problem today which I think is similar:

julia> mutable struct A
           id::Int
       end

julia> sa = StructArray{A}(undef, 0)
0-element StructArray(::Vector{Int64}) with eltype A

julia> push!(sa, A(1))
1-element StructArray(::Vector{Int64}) with eltype A:
 A(1)

## getting

julia> sa[1]
A(1)

julia> sa[1].id
1

julia> view(sa, 1).id
0-dimensional view(::Vector{Int64}, 1) with eltype Int64:
1

## setting

julia> sa[1].id = 2 # fails silently
2

julia> sa
1-element StructArray(::Vector{Int64}) with eltype A:
 A(1)

julia> sa.id[1] = 2 # succeeds
2

julia> sa
1-element StructArray(::Vector{Int64}) with eltype A:
 A(2)

Am I right in assuming that these problems are connected? If not, please correct me if I'm wrong. :)

For comparison, here's how it works in a vector of structs:

julia> a = [A(1)]
1-element Vector{A}:
 A(1)

## getting

julia> a[1]
A(1)

julia> a[1].id
1

## setting

julia> a[1].id = 2 # succeeds
2

julia> a
1-element Vector{A}:
 A(2)

I had assumed that typeof(sa[1]) == typeof(a[1]) == A and therefore their reaction to setting properties would be the same.

@piever
Copy link
Collaborator

piever commented Apr 21, 2021

That's actually a separate issue, as the structs are not actually stored (only their fields are), so sa[1] creates the struct on the fly. As a consequence, modifying does not modify the array (see also this comment #181 (comment)).

Maybe that behavior should be documented more thoroughly (documentation PRs welcome!)

@fbanning
Copy link

fbanning commented Apr 22, 2021

the structs are not actually stored (only their fields are), so sa[1] creates the struct on the fly

I did not know that! Thanks for the answer. :) I will have to look for other ways around this then, maybe LazyRow will do what I hope for.

timholy added a commit that referenced this issue May 8, 2022
Fixes #131
Closes #184

Co-authored-by: Gustavo Goretkin <[email protected]>
sjdaines added a commit to PALEOtoolkit/PALEOboxes.jl that referenced this issue May 27, 2022
This is needed for StructArrays v0.6.8 https://github.com/JuliaArrays/StructArrays.jl/releases/tag/v0.6.8
See
JuliaArrays/StructArrays.jl#131
JuliaArrays/StructArrays.jl#216

It's not clear to me whether the behaviour of StructArrays.jl is now correct (so this is a PALEOboxes.jl fix that
used to not matter) or whether this is a workaround for a newly introduced StructArrays.jl issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants