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

Parseable repr for undef #33211

Merged
merged 4 commits into from
Sep 12, 2019
Merged

Conversation

nickrobinson251
Copy link
Contributor

@nickrobinson251 nickrobinson251 commented Sep 10, 2019

julia> undef
array initializer with undefined values
  • But gives us a parseable representation if requested (with show/repr) -- which is the motivation for the change
julia> repr(undef)
"UndefInitializer()"
  • To me (others may disagree), a bonus is we get more familiar Julia syntax in the first case here, which helps to better distinguishes the two cases:
julia> fill(undef, 2)  # thing you probably don't want - Vector of Initializers
2-element Array{UndefInitializer,1}:
 UndefInitializer()
 UndefInitializer()

julia> Vector(undef, 2)  # thing you probably wanted - Vector of undefined values
2-element Array{Any,1}:
 #undef
 #undef

@nickrobinson251 nickrobinson251 changed the title WIP: Restrict verbose undef show to MIME"text/plain" Parseable repr for undef Sep 10, 2019
@nickrobinson251
Copy link
Contributor Author

I'd appreciate being advised on:

  • do the tests here seem reasonable? can/should I test differently?
  • does this need news? (fwiw, no tests needed updating, but it's definitely public facing)

@JeffBezanson
Copy link
Member

Oops, just commented on exactly this on your other PR. Sorry, a bit behind on PRs and issues :)

@JeffBezanson
Copy link
Member

Looks good to me. I think the only question is whether to output UndefInitializer() (which is perfectly clear, and sufficiently scary 😄 ) or undef, which is how it's usually entered.

@JeffBezanson JeffBezanson added display and printing Aesthetics and correctness of printed representations of objects. triage This should be discussed on a triage call labels Sep 10, 2019
@nickrobinson251
Copy link
Contributor Author

I think the only question is whether to output UndefInitializer() (which is perfectly clear, and sufficiently scary 😄 ) or undef, which is how it's usually entered.

I slightly prefer UndefInitializer() over undef mostly because it is visually more different to #undef

and secondly because it is clearer (in the fill example) that these are "Initializers" (having the canonical "create an instance of a type" syntax) not "undefined values".

Also this seemed to be prefered by folks at #33204 (comment).

...but i do not feel strongly either way.

@JeffBezanson
Copy link
Member

Ok, I'm convinced :)

base/show.jl Outdated Show resolved Hide resolved
@nickrobinson251
Copy link
Contributor Author

This PR now results in:

julia> undef
UndefInitializer(): array initializer with undefined values

julia> repr(undef)
"UndefInitializer()"

julia> fill(undef, 2)
2-element Array{UndefInitializer,1}:
 UndefInitializer()
 UndefInitializer()

julia> Vector(undef, 2)
2-element Array{Any,1}:
 #undef
 #undef

@JeffBezanson JeffBezanson merged commit 4887c49 into JuliaLang:master Sep 12, 2019
@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Sep 12, 2019
@StefanKarpinski StefanKarpinski added the needs news A NEWS entry is required for this change label Sep 12, 2019
@StefanKarpinski
Copy link
Member

Should probably have a NEWS entry.

@nickrobinson251
Copy link
Contributor Author

Should probably have a NEWS entry.

Ah, sorry! Since this was already merged, there's a separate PR to add news: #33254

@nickrobinson251 nickrobinson251 deleted the npr/undef-repr branch September 13, 2019 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display and printing Aesthetics and correctness of printed representations of objects. needs news A NEWS entry is required for this change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

undef does not have parseable repr
4 participants