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

SubArray getindex doesn't work on immutable array types #10001

Closed
jakebolewski opened this issue Feb 1, 2015 · 6 comments
Closed

SubArray getindex doesn't work on immutable array types #10001

jakebolewski opened this issue Feb 1, 2015 · 6 comments

Comments

@jakebolewski
Copy link
Member

Should this method be renamed to sortrows! / sortcols? It currently fails for AbstractMatrix subtypes which do not define setindex!.

Ex.

julia> d = Diagonal(rand(10,10))
10x10 Base.LinAlg.Diagonal{Float64}:
 0.633063  0.0       0.0       0.0        0.0       0.0       0.0       0.0       0.0       0.0
 0.0       0.719781  0.0       0.0        0.0       0.0       0.0       0.0       0.0       0.0
 0.0       0.0       0.779313  0.0        0.0       0.0       0.0       0.0       0.0       0.0
 0.0       0.0       0.0       0.0480023  0.0       0.0       0.0       0.0       0.0       0.0
 0.0       0.0       0.0       0.0        0.747617  0.0       0.0       0.0       0.0       0.0
 0.0       0.0       0.0       0.0        0.0       0.188713  0.0       0.0       0.0       0.0
 0.0       0.0       0.0       0.0        0.0       0.0       0.572225  0.0       0.0       0.0
 0.0       0.0       0.0       0.0        0.0       0.0       0.0       0.840128  0.0       0.0
 0.0       0.0       0.0       0.0        0.0       0.0       0.0       0.0       0.317382  0.0
 0.0       0.0       0.0       0.0        0.0       0.0       0.0       0.0       0.0       0.688265

julia> sortrows(d)
ERROR: MethodError: `getindex` has no method matching getindex(::Base.LinAlg.Diagonal{Float64}, ::Array{Int64,1}, ::UnitRange{Int64})
Closest candidates are:
  getindex(::AbstractArray{T,2}, ::Any, ::Any, ::Any, ::Any...)
  getindex{T,N,P,IV}(::SubArray{T,N,P,IV,LD}, ::Union(Array{Int64,1},Range{Int64},Colon,Int64)...)
  getindex{T,N,P,IV}(::SubArray{T,N,P,IV,LD}, ::Union(AbstractArray{T,1},Real), ::Union(AbstractArray{T,1},Real))
  ...

 in sortrows at sort.jl:370

julia> s = sub(d, 1:5, 1:5)
5x5 SubArray{Float64,2,Base.LinAlg.Diagonal{Float64},(UnitRange{Int64},UnitRange{Int64}),0}:
 0.633063  0.0       0.0       0.0        0.0
 0.0       0.719781  0.0       0.0        0.0
 0.0       0.0       0.779313  0.0        0.0
 0.0       0.0       0.0       0.0480023  0.0
 0.0       0.0       0.0       0.0        0.747617

julia> sortrows(s)
ERROR: MethodError: `setindex!` has no method matching setindex!(::Base.LinAlg.Diagonal{Float64}, ::Float64, ::Int64, ::Int64)
Closest candidates are:
  setindex!(::AbstractArray{T,2}, ::Any, ::Any, ::Any, ::Any, ::Any...)
  setindex!{T}(::Array{T,N}, ::Any, ::Real, ::Real)
  setindex!{T}(::Array{T,N}, ::Any, ::Real, ::Real, ::Real)
  ...
@simonster
Copy link
Member

The line of sortrows that's throwing is just A[p,:], where p::Vector{Int}. I think it's reasonable to expect this to work, but it doesn't for Diagonal:

julia> d = Diagonal(rand(10,10));

julia> d[[1, 2, 3, 4, 5], :]
ERROR: MethodError: `getindex` has no method matching getindex(::Base.LinAlg.Diagonal{Float64}, ::Array{Int64,1}, ::UnitRange{Int64})
Closest candidates are:
  getindex(::AbstractArray{T,2}, ::Any, ::Any, ::Any, ::Any...)
  getindex{T,N,P,IV}(::SubArray{T,N,P,IV,LD}, ::Union(Array{Int64,1},Range{Int64},Int64,Colon)...)
  getindex{T,N,P,IV}(::SubArray{T,N,P,IV,LD}, ::Union(AbstractArray{T,1},Real), ::Union(AbstractArray{T,1},Real))
  ...


julia> s = sub(d, 1:5, 1:5);

julia> s[[1, 2, 3, 4, 5], :]
ERROR: MethodError: `setindex!` has no method matching setindex!(::Base.LinAlg.Diagonal{Float64}, ::Float64, ::Int64, ::Int64)
Closest candidates are:
  setindex!(::AbstractArray{T,2}, ::Any, ::Any, ::Any, ::Any, ::Any...)
  setindex!{T}(::Array{T,N}, ::Any, ::Real, ::Real)
  setindex!{T}(::Array{T,N}, ::Any, ::Real, ::Real, ::Real)
  ...

 in copy! at multidimensional.jl:436
 in getindex at subarray2.jl:82

@JeffBezanson
Copy link
Sponsor Member

Yes, the "mutation" is an implementation of copy that works by calling copy! from the existing array to a new similar one.

@JeffBezanson JeffBezanson changed the title Should sortrows / sortcols mutate the underlying Matrix or return a copy? SubArray getindex doesn't work on immutable array types Feb 1, 2015
@mbauman
Copy link
Sponsor Member

mbauman commented Jun 4, 2015

This seems to be evidence that the solution to #10004 should be to provide a full array from similar.

@tkelman
Copy link
Contributor

tkelman commented Jun 4, 2015

provide a full array from similar.

From similar of what, a Diagonal or a subarray of a Diagonal? For the former I don't like the idea of promoting to dense, for the latter I'm not sure what would be best to do.

@mbauman
Copy link
Sponsor Member

mbauman commented Jun 4, 2015

(response in #11574)

@mbauman
Copy link
Sponsor Member

mbauman commented Feb 17, 2016

Fixed by #11582. All the examples above look much better now:

julia> d = Diagonal(rand(10,10));

julia> s = sub(d, 1:5, 1:5);

julia> sortrows(d)
ERROR: ArgumentError: cannot set an off-diagonal index (10, 1) to a nonzero value (0.05324786837334572)

julia> sortrows(s)
ERROR: ArgumentError: cannot set an off-diagonal index (5, 1) to a nonzero value (0.05324786837334572)

julia> d[[1, 2, 3, 4, 5], :]
ERROR: ArgumentError: diagonal matrix must be square

julia> s[[1, 2, 3, 4, 5], :]
5x5 Diagonal{Float64}:
 0.160014                               
          0.574236                      
                   0.0339231            
                             0.151776   
                                      0.509368

@mbauman mbauman closed this as completed Feb 17, 2016
This issue was closed.
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

5 participants