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

Some fixes for summary, show, and map for non-1 indices #17729

Merged
merged 3 commits into from
Aug 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ indices{T,N}(A::AbstractArray{T,N}, d) = d <= N ? indices(A)[d] : OneTo(1)

Returns the tuple of valid indices for array `A`.
"""
function indices{T,N}(A::AbstractArray{T,N})
@_inline_pure_meta
function indices(A)
@_inline_meta
map(s->OneTo(s), size(A))
end

Expand All @@ -50,6 +50,7 @@ end
# comes up in other applications.
indices1{T}(A::AbstractArray{T,0}) = OneTo(1)
indices1{T}(A::AbstractArray{T}) = (@_inline_meta; indices(A)[1])
indices1(iter) = OneTo(length(iter))

unsafe_indices(A) = indices(A)
unsafe_indices(r::Range) = (OneTo(unsafe_length(r)),) # Ranges use checked_sub for size
Expand All @@ -76,6 +77,7 @@ ndims{T,N}(::Type{AbstractArray{T,N}}) = N
ndims{T<:AbstractArray}(::Type{T}) = ndims(supertype(T))
length(t::AbstractArray) = prod(size(t))
_length(A::AbstractArray) = prod(map(unsafe_length, indices(A))) # circumvent missing size
_length(A) = length(A)
endof(a::AbstractArray) = length(a)
first(a::AbstractArray) = a[first(eachindex(a))]

Expand Down
14 changes: 9 additions & 5 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@ import Core: arraysize, arrayset, arrayref
size(a::Array, d) = arraysize(a, d)
size(a::Vector) = (arraysize(a,1),)
size(a::Matrix) = (arraysize(a,1), arraysize(a,2))
size(a::Array) = _size((), a)
size(a::Array) = (@_inline_meta; _size((), a))
_size{_,N}(out::NTuple{N}, A::Array{_,N}) = out
_size{_,M,N}(out::NTuple{M}, A::Array{_,N}) = _size((out..., size(A,M+1)), A)
function _size{_,M,N}(out::NTuple{M}, A::Array{_,N})
@_inline_meta
_size((out..., size(A,M+1)), A)
end

asize_from(a::Array, n) = n > ndims(a) ? () : (arraysize(a,n), asize_from(a, n+1)...)

Expand Down Expand Up @@ -220,7 +223,7 @@ end
# make a collection similar to `c` and appropriate for collecting `itr`
_similar_for(c::AbstractArray, T, itr, ::SizeUnknown) = similar(c, T, 0)
_similar_for(c::AbstractArray, T, itr, ::HasLength) = similar(c, T, Int(length(itr)::Integer))
_similar_for(c::AbstractArray, T, itr, ::HasShape) = similar(c, T, convert(Dims,size(itr)))
_similar_for(c::AbstractArray, T, itr, ::HasShape) = similar(c, T, indices(itr))
_similar_for(c, T, itr, isz) = similar(c, T)

"""
Expand Down Expand Up @@ -286,8 +289,9 @@ function _collect(c, itr, ::EltypeUnknown, isz::Union{HasLength,HasShape})
end

function collect_to_with_first!(dest::AbstractArray, v1, itr, st)
dest[1] = v1
return collect_to!(dest, itr, 2, st)
i1 = first(linearindices(dest))
dest[i1] = v1
return collect_to!(dest, itr, i1+1, st)
end

function collect_to_with_first!(dest, v1, itr, st)
Expand Down
1 change: 1 addition & 0 deletions base/generator.jl
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ iteratorsize{T<:AbstractArray}(::Type{T}) = HasShape()
iteratorsize{I,F}(::Type{Generator{I,F}}) = iteratorsize(I)
length(g::Generator) = length(g.iter)
size(g::Generator) = size(g.iter)
indices(g::Generator) = indices(g.iter)
ndims(g::Generator) = ndims(g.iter)

iteratoreltype{I,T}(::Type{Generator{I,T}}) = EltypeUnknown()
19 changes: 10 additions & 9 deletions base/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1476,13 +1476,16 @@ e.g. `10-element Array{Int64,1}`.
summary(x) = string(typeof(x)) # e.g. Int64

# sizes such as 0-dimensional, 4-dimensional, 2x3
dims2string(d) = isempty(d) ? "0-dimensional" :
length(d) == 1 ? "$(d[1])-element" :
join(map(string,d), '×')
dims2string(d::Dims) = isempty(d) ? "0-dimensional" :
length(d) == 1 ? "$(d[1])-element" :
join(map(string,d), '×')

inds2string(inds::Indices) = join(map(string,inds), '×')

# anything array-like gets summarized e.g. 10-element Array{Int64,1}
summary(a::AbstractArray) =
string(dims2string(size(a)), " ", typeof(a))
summary(a::AbstractArray) = _summary(a, to_shape(indices(a)))
_summary(a, dims::Dims) = string(dims2string(dims), " ", typeof(a))
_summary(a, inds) = string(typeof(a), " with indices ", inds2string(inds))

# n-dimensional arrays
function show_nd(io::IO, a::AbstractArray, print_matrix, label_slices)
Expand Down Expand Up @@ -1657,17 +1660,15 @@ function show_vector(io::IO, v, opn, cls)
io = IOContext(io, :compact => compact)
end
print(io, prefix)
if limited && length(v) > 20
inds = _indices1(v)
if limited && _length(v) > 20
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is _length? Could we use size(v, 1) instead?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inds = indices1(v)
show_delim_array(io, v, opn, ",", "", false, inds[1], inds[1]+9)
print(io, " \u2026 ")
show_delim_array(io, v, "", ",", cls, false, inds[end-9], inds[end])
else
show_delim_array(io, v, opn, ",", cls, false)
end
end
_indices1(v::AbstractArray) = indices(v,1)
_indices1(iter) = 1:length(iter)

# printing BitArrays

Expand Down
25 changes: 24 additions & 1 deletion test/offsetarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ Base.size(A::OffsetArray) = errmsg(A)
Base.size(A::OffsetArray, d) = errmsg(A)
Base.eachindex(::LinearSlow, A::OffsetArray) = CartesianRange(indices(A))
Base.eachindex(::LinearFast, A::OffsetVector) = indices(A, 1)
Base.summary(A::OffsetArray) = string(typeof(A))*" with indices "*string(indices(A))

# Implementations of indices and indices1. Since bounds-checking is
# performance-critical and relies on indices, these are usually worth
Expand Down Expand Up @@ -195,6 +194,9 @@ show(io, v)
str = takebuf_string(io)
show(io, parent(v))
@test str == takebuf_string(io)
smry = summary(v)
@test contains(smry, "OffsetArray{Float64,1")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closing } omission intentional?

Copy link
Sponsor Member Author

@timholy timholy Jul 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The third parameter is the "parent" array type, and I don't care about that here.

@test contains(smry, "with indices -1:1")
function cmp_showf(printfunc, io, A)
ioc = IOContext(io, limit=true, compact=true)
printfunc(ioc, A)
Expand All @@ -207,6 +209,23 @@ cmp_showf(Base.print_matrix, io, OffsetArray(rand(5,5), (10,-9))) # rows&c
cmp_showf(Base.print_matrix, io, OffsetArray(rand(10^3,5), (10,-9))) # columns fit
cmp_showf(Base.print_matrix, io, OffsetArray(rand(5,10^3), (10,-9))) # rows fit
cmp_showf(Base.print_matrix, io, OffsetArray(rand(10^3,10^3), (10,-9))) # neither fits
targets1 = ["0-dimensional OAs.OffsetArray{Float64,0,Array{Float64,0}}:\n1.0",
"OAs.OffsetArray{Float64,1,Array{Float64,1}} with indices 2:2:\n 1.0",
"OAs.OffsetArray{Float64,2,Array{Float64,2}} with indices 2:2×3:3:\n 1.0",
"OAs.OffsetArray{Float64,3,Array{Float64,3}} with indices 2:2×3:3×4:4:\n[:, :, 4] =\n 1.0",
"OAs.OffsetArray{Float64,4,Array{Float64,4}} with indices 2:2×3:3×4:4×5:5:\n[:, :, 4, 5] =\n 1.0"]
targets2 = ["(1.0,1.0)",
"([1.0],[1.0])",
"(\n[1.0],\n\n[1.0])",
"(\n[1.0],\n\n[1.0])",
"(\n[1.0],\n\n[1.0])"]
for n = 0:4
a = OffsetArray(ones(Float64,ntuple(d->1,n)), ntuple(identity,n))
show(IOContext(io, limit=true), MIME("text/plain"), a)
@test takebuf_string(io) == targets1[n+1]
show(IOContext(io, limit=true), MIME("text/plain"), (a,a))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the outputs here be tested?

@test takebuf_string(io) == targets2[n+1]
end

# Similar
B = similar(A, Float32)
Expand Down Expand Up @@ -303,6 +322,10 @@ map!(+, dest, am, am)
@test dest[1,8] == 4
@test dest[1,9] == -2

am = map(identity, a)
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrap this in a let block

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, seems this whole file is written at global scope. that prevents gc from ever cleaning up the intermediate values. maybe we should defer this work and add this to various @testsets once kshyatt's PR is merged.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole file is inside a let:

@test isa(am, OffsetArray)
@test am == a

A = OffsetArray(rand(4,4), (-3,5))
@test maximum(A) == maximum(parent(A))
@test minimum(A) == minimum(parent(A))
Expand Down