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

view != getindex, with IdentityUnitRange #100

Closed
mcabbott opened this issue Feb 27, 2020 · 4 comments
Closed

view != getindex, with IdentityUnitRange #100

mcabbott opened this issue Feb 27, 2020 · 4 comments
Assignees

Comments

@mcabbott
Copy link

I got some strange results copying a piece into a larger array, and if I understand right it looks like a problem with how view deals with IdentityUnitRange.

julia> out = reshape(1:15, 5,3) .+ 0; 

julia> item1 = 101:100:301;

julia> out[axes(item1)..., 1] .= item1; 

julia> out 
5×3 Array{Int64,2}:
 101   6  11
 201   7  12
 301   8  13
   4   9  14
   5  10  15

julia> item = OffsetArray(102:100:302, +2)
102:100:302 with indices 3:5

julia> out[axes(item)..., 1] # looks correct  
3-element OffsetArray(::Array{Int64,1}, 3:5) with eltype Int64 with indices 3:5:
 301
   4
   5

julia> view(out, axes(item)..., 1) # wrong? 
3-element view(::Array{Int64,2}, :, 1) with eltype Int64 with indices 3:5:
 5
 6
 7

julia> out[axes(item)..., 2] .= item;

julia> copyto!(view(out, axes(item)..., 2), item); # same effect

julia> out # values of item are in the wrong places!
5×3 Array{Int64,2}:
 101    6  202
 201    7  302
 301    8   13
   4    9   14
   5  102   15

(@v1.5) pkg> st OffsetArrays
Status `~/.julia/environments/v1.5/Project.toml`
  [6fe1bfb0] OffsetArrays v0.11.4
@johnnychen94
Copy link
Member

johnnychen94 commented Feb 27, 2020

If you wrap it with CartesianIndices, it works as expected...

julia> view(out, CartesianIndices(item), 1)
3-element view(::Array{Int64,2}, [3, 4, 5], 1) with eltype Int64:
 301
   4
   5

julia> to_indices(item, (axes(item)..., 1))
(3:5, 1)

julia> to_indices(item, (CartesianIndices(item), 1))
(CartesianIndex{1}[CartesianIndex(3,), CartesianIndex(4,), CartesianIndex(5,)], 1)

@mcabbott
Copy link
Author

Thanks, that's a useful work-around, which I would never have thought to try!

@johnnychen94
Copy link
Member

johnnychen94 commented Mar 4, 2020

update with another result:

julia> ind = OffsetArrays.IdOffsetRange(3:5, 0)
3:5

julia> view(out, ind, 1)
3-element view(::Array{Int64,2}, 3:5, 1) with eltype Int64 with indices 3:5:
 5
 6
 7

julia> view(out, 3:5, 1)
3-element view(::Array{Int64,2}, 3:5, 1) with eltype Int64:
 301
   4
   5

Definitely that there's something not covered here. Let me see if I can tackle this ( seems like a good exercise to me :P)

@johnnychen94
Copy link
Member

For the usage of OffsetArrays, this issue is fixed by #101

As @timholy indicated, we should also give a patch in Base so that other types of index-preserving ranges don't need such a fix. Hence I'll keep this issue open until I find some time for that.

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

2 participants