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

Compact show for SArray and MArray #906

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jishnub
Copy link
Member

@jishnub jishnub commented May 7, 2021

Along the lines of JuliaLang/julia#40722, this changes the way show(io, ::SArray) and show(io, ::MArray) displays the contents. The type information printed as a prefix distinguishes these from other dense arrays. Also makes the output valid as a constructor.

After this PR:

julia> show(SVector{2, Int64}((1, 2)))
SVector{2, Int64}((1, 2))

julia> show(MVector{2, Int64}((1, 2)))
MVector{2, Int64}((1, 2))

Edit: Closes #692

@mcabbott
Copy link
Collaborator

mcabbott commented May 8, 2021

Hah I was going to have a go at this too, JuliaArrays/FillArrays.jl#144 is the other one...

But I think they can be more compact, at least sometimes. Why not print SA[1, 2, 3]? Where I got lost was in how Base decides whether to print a type in front... I see now there are forms SA_F32[1, 2] and SA{Int32}[1, 2] which would be ideal.

julia> (rand(Int32.(1:99),999),)
(Int32[60, 26, 91, 10, 70, 33, 25, 64, 44, 64  …  20, 15, 21, 6, 39, 67, 18, 18, 84, 47],)

Less sure about the M case, but perhaps MVector(1, 2) is nicer?

@andyferris
Copy link
Member

andyferris commented May 10, 2021

Did we consider SVector{2}([1, 2])? If the eltype isn't obvious it can go on the inner array, like SVector{2}(Any[1, 2]), so effectively reusing the logic applied to Vector in a composable way.

@mcabbott
Copy link
Collaborator

mcabbott commented May 10, 2021

Here's a quick attempt to hook into the machinery... @less Base.typeinfo_prefix(stdout, [1,2,3]) is where it decides what prefix to print on other arrays, thus:

julia> function Base.typeinfo_prefix(io::IO, X::SArray)
           typeinfo = get(io, :typeinfo, Any)::Type
           if !(X isa typeinfo)
               typeinfo = Any
           end
           # what the context already knows about the eltype of X:
           eltype_ctx = Base.typeinfo_eltype(typeinfo)
           eltype_X = eltype(X)

               # Types hard-coded here are those which are created by default for a given syntax
               if eltype_X == eltype_ctx
                   "SA", false
               elseif eltype_X == Float32  # added special case
                   "SA_F32", false
               elseif !isempty(X) && Base.typeinfo_implicit(eltype_X)
                   "SA", true
               # elseif print_without_params(eltype_X)  # not sure what this does!
               #    sprint(show_type_name, unwrap_unionall(eltype_X).name), false # Print "Array" rather than "Array{T,N}"
               else
                   string("SA{", eltype_X, "}"), false
               end
       end

julia> (SA[1,2,3], SA{Int32}[1 2; 3 4], SA[1f0, 2f0])
(SA[1, 2, 3], SA{Int32}[1 2; 3 4], SA_F32[1.0, 2.0])

julia> (SA{Int}[], SA{Float64}[], SA[])  # empty
(SA{Int64}[], SA{Float64}[], SA{Union{}}[])

julia> (SVector{2}(Any[1, 2]), SA{Any}[1,2], SVector{3}(Any[1, :a, nothing]))  # first really does have eltype Int
(SA[1, 2], SA{Any}[1, 2], SA{Any}[1, :a, nothing])

julia> repr(@SMatrix rand(10,10); context=(:limit => true, :compact => true))
"SA[0.262792 0.33571 … 0.432945 0.18245; 0.535286 0.995272 … 0.881322 0.00517454; … ; 0.908566 0.738296 … 0.0315535 0.46813; 0.602212 0.843338 … 0.209644 0.31736]"

julia> [SA[0f0,i] for i in 1:3]  # not sure if this should omit eltype? Omit everything? When :compact => true?
3-element Vector{SVector{2, Float32}}:
 SA[0.0, 1.0]
 SA[0.0, 2.0]
 SA[0.0, 3.0]

julia> ([SA[1f0,2f0], SA[3f0,4f0]], [SA[1f0,2f0], SA[3f0,4f0,5f0]]) # arrays of arrays
(SVector{2, Float32}[[1.0, 2.0], [3.0, 4.0]], SArray{S, Float32, 1} where S<:Tuple[SA[1.0, 2.0], SA[3.0, 4.0, 5.0]])

@andyferris
Copy link
Member

I'm wondering if SA is would be confusing for people who aren't direct StaticArrays.jl users. We are used by a lot of downstream packages and general Julia users may receive static arrays (I think I saw some from Plots.jl or somewhere like that recently) and be presented (by show) with SA[1,2,3] and therefore expect the element type to be SA.

I realize it is shorter and convenient for developers, but am unconvinced that it's the best choice for show or repr.

@mcabbott
Copy link
Collaborator

Much as I like SA, it is a bit of a syntax trick.

You could still minimise down to SVector(1.0, 2.0, 3.0) when the eltype is obvious. But SMatrix{2,2}(1,2,3,4) is confusing as it's [1 3; 2 4]. If you wrap an array, I'm a bit surprised that SVector{2}(Any[1, 2]) doesn't work, I suppose it's like (Any[1,2]...,), so at least for abstract types I guess you'd have to write it in the curly brackets.

@andyferris
Copy link
Member

I'm a bit surprised that SVector{2}(Any[1, 2]) doesn't work

Oh, so am I! Even convert is broken, and neither match Base.

julia> SVector{2}(Any[1,2])
2-element SVector{2, Int64} with indices SOneTo(2):
 1
 2

julia> convert(SVector{2}, Any[1,2])
2-element SVector{2, Int64} with indices SOneTo(2):
 1
 2

julia> Vector(Any[1,2])
2-element Vector{Any}:
 1
 2

julia> convert(Vector, Any[1,2])
2-element Vector{Any}:
 1
 2

We should definitely this behavior - so don't feel afraid to implement show this way.

@mcabbott
Copy link
Collaborator

Would you ever want an abstract type though? Any[1] you can write 2.5 into, maybe you could want that for an MArray? SVector{2}(Any[1,"β"]) does preserve it, of course.

@andyferris
Copy link
Member

andyferris commented May 11, 2021

In my opinion, it's important to mimic Base as closely as possible, to help people write generic code and avoid surprises when using StaticArrays.

While this is more important for MArray due to setindex!, there's also the common issue of arrays with eltype Union{T, missing} and making life easy for inference in these cases, or conforming to some storage container which you want store and retrieve such values (like with a Vector{SVector{3, Union{Float64, Missing}}} - individual elements could be entirely Float64 or entirely Missing or a mixture).

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

Successfully merging this pull request may close these issues.

repr roundtripping. "print it like you build it"
3 participants