Skip to content

propagate context through a few more printing functions#46716

Merged
aviatesk merged 1 commit intomasterfrom
kc/context_propagate
Sep 13, 2022
Merged

propagate context through a few more printing functions#46716
aviatesk merged 1 commit intomasterfrom
kc/context_propagate

Conversation

@KristofferC
Copy link
Copy Markdown
Member

@KristofferC KristofferC commented Sep 12, 2022 •
edited
Loading

makes :compact=>false a way to avoid the expensive typealias checking (which seems to be what's desired looking at

julia/base/show.jl

Lines 954 to 960 in 7e51510

elseif get(io, :compact, true) && show_typealias(io, x)
return
elseif x isa DataType
show_datatype(io, x)
return
elseif x isa Union
if get(io, :compact, true) && show_unionaliases(io, x)
.

Before:

julia> d = Dict{String, Any}()

julia> @btime repr($d; context=:compact=>false);
  182.768 μs (35 allocations: 27.10 KiB)

After:

julia> d = Dict{String, Any}()

julia> @btime repr($d; context=:compact=>false);
  1.236 μs (22 allocations: 904 bytes)

I don't really know how the typealias printing works so not sure how to add a test.

propagate context through a few more printing functions
4633dd5 
makes `:compact=>false` a way to avoid the expensive typealias checking
@KristofferC KristofferC requested a review from vtjnash September 12, 2022 14:45
vtjnash
vtjnash approved these changes Sep 12, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

yes, this seems to be an obvious bugfix

aviatesk
aviatesk approved these changes Sep 13, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@aviatesk aviatesk left a comment

Choose a reason for hiding this comment

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

Test failures are unrelated.

@aviatesk aviatesk merged commit b284bc6 into master Sep 13, 2022
@aviatesk aviatesk deleted the kc/context_propagate branch September 13, 2022 03:04
@DilumAluthge DilumAluthge mentioned this pull request Sep 28, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vtjnash vtjnash vtjnash approved these changes

@aviatesk aviatesk aviatesk approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@KristofferC @vtjnash @aviatesk