-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Improve show
for 0-dim array
#33206
Improve show
for 0-dim array
#33206
Conversation
@KristofferC makes the good point that
So for these cases, what should we print? one option is something that doesn't parse, e.g. |
is just an unitialized 0 dimensional array: julia> a = Array{Any,0}(undef)
0-dimensional Array{Any,0}:
#undef
julia> a[]
ERROR: UndefRefError: access to undefined reference
Stacktrace:
is an initialized 0-dimensional array with the uninitialized-array initializer in it. julia> b = fill(undef)
0-dimensional Array{UndefInitializer,0}:
array initializer with undefined values
julia> b[]
array initializer with undefined values |
okay, how do things look now? we have julia> x = fill(undef)
0-dimensional Array{UndefInitializer,0}:
array initializer with undefined values
julia> y = Array{Any,0}(undef)
0-dimensional Array{Any,0}:
#undef
julia> repr(x)
"fill(array initializer with undefined values)"
julia> repr(y)
"fill(#undef)" Edit: julia> repr(x)
"array initializer with undefined values"
julia> repr(y)
"#undef" this seems consistent and reasonable to me - but keen to hear opinions :) Thanks for the help @KristofferC ! |
show
for 0-dim arrayshow
for 0-dim array
there was one related test failure, where the old output was hard coded Error in testset offsetarray:
Test Failed at /Users/julia/buildbot/worker-tabularasa/tester_macos64/build/share/julia/test/offsetarray.jl:223
Expression: String(take!(io)) == targets2[n + 1]
Evaluated: "(fill(1.0), fill(1.0))" == "(1.0, 1.0)" So I have updated that test :) |
It really seems to me that |
Adding to that, it looks like the existing show method for |
Yep, totally agree on all these points 👍 As i think you've seen, I made the |
A couple issues from triage:
|
Yeah, sounds good to me 👍 Think it adds a small bit of extra complexity but means we should get |
On second thoughts, how does one construct a |
4b99f62
to
3049171
Compare
After the latest commit (and rebased to include #33211) we have: julia> for x in (:(zeros(Int32)), :(collect('b')), :(fill(nothing)), :(BitArray(0)), :(fill(undef)), :(Array{String, 0}(undef)), :(Array{Int32, 0}(undef)))
println("input: ", x)
println("show: ", eval(x))
println()
end
input: zeros(Int32)
show: fill(0)
input: collect('b')
show: fill('b')
input: fill(nothing)
show: fill(nothing)
input: BitArray(0)
show: fill(false)
input: fill(undef)
show: fill(UndefInitializer())
input: Array{String, 0}(undef)
show: Array{String,0}(UndefInitializer())
input: Array{Int32, 0}(undef)
show: fill(210687536) As the julia> x = BitArray(0)
0-dimensional BitArray{0}:
0
julia> repr(x)
"fill(false)"
julia> x == eval(Meta.parse(repr(x)))
true
julia> x === eval(Meta.parse(repr(x)))
false julia> struct MyZeroDimArray{T} <: AbstractArray{T, 0}
data::Array{T, 0}
end
julia> Base.size(A::MyZeroDimArray) = size(A.data)
julia> Base.getindex(A::MyZeroDimArray, indices...) = A.data[indices...]
julia> x = ZeroDimArray(fill(0))
0-dimensional MyZeroDimArray{Int64}:
0
julia> repr(x)
"fill(0)"
julia> x == eval(Meta.parse(repr(x)))
true
julia> x === eval(Meta.parse(repr(x)))
false I think this is fine, since we cannot know what is a valid constructor for these types, e.g. Please let me know how this seems, and if I can improve things further :) |
91dcd46
to
6e4f45e
Compare
bump :) |
6e4f45e
to
98c4bcd
Compare
(^just resolving merge conflicts in NEWS) |
(The "triage" label means that it is flagged to get a decision soon, no need to bump.) |
Triage says merge when CI is done. |
Thanks for all the help on this! |
WIP: opening for comments / see if this is the right approach (printing is confusing...)needs testBefore (on version 1.2.0):
this PR: [EDIT: updated below]