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.

Sign up for GitHub

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

Jump to bottom

Consider redacting printing of Axis types #172

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+13 −1
Open

Consider redacting printing of Axis types #172

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/show.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Show AbstractAxis types
# Show AbstractAxis instances
Base.show(io::IO, ::MIME"text/plain", ::Axis{IdxMap}) where IdxMap = print(io, "Axis$IdxMap")
Base.show(io::IO, ::Axis{IdxMap}) where IdxMap = print(io, "Axis$IdxMap")

Expand All @@ -22,9 +22,21 @@ Base.show(io::IO, ::ViewAxis{Inds, IdxMap, <:Ax}) where {Inds, IdxMap, Ax} =
Base.show(io::IO, ::ViewAxis{Inds, IdxMap, <:NullorFlatAxis}) where {Inds, IdxMap} =
print(io, Inds)

# Show reducted Axis types to avoid excessive stacktraces
Base.show(io::IO, ::Type{Axis{IdxMap}}) where {IdxMap} = print(io, "Axis{...}")
Base.show(io::IO, ::Type{FlatAxis}) = print(io, "FlatAxis")
Base.show(io::IO, ::Type{NullAxis}) = print(io, "NullAxis")
function Base.show(io::IO, ::Type{PartitionedAxis{PartSz,IdxMap,Ax}}) where {PartSz,IdxMap,Ax}
return print(io, "PartitionedAxis{$PartSz, {...}, $Ax}")
end
Base.show(io::IO, ::Type{ShapedAxis{Shape,IdxMap}}) where {Shape,IdxMap} = print(io, "ShapedAxis($Shape, {...})")
Base.show(io::IO, ::Type{ViewAxis{Inds,IdxMap,Ax}}) where {Inds,IdxMap,Ax} = print(io, "ViewAxis{$Inds, {...}, $Ax)")
Copy link
Owner

Choose a reason for hiding this comment

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

This is how ComponentArrays used to do type printing, but we had to get rid of it because it tends to break a lot of things

Copy link
Owner

Choose a reason for hiding this comment

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

Base.print_type_stacktrace was supposed to be the solution to this, but if it’s not working anymore, I’m not sure what to do. We don’t want to potentially cause a segfault for someone.

Copy link
Contributor Author

@nrontsis nrontsis Oct 27, 2022 •
edited

Choose a reason for hiding this comment

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

Ah, I should have thought that I am reinventing the wheel here 😄

I took a look at the current Julia's show code that is used for stacktraces but I found it too complicated, and it made me think that overriding something "private" there might be dangerous (for example, I generated too many StackOverflows while playing around with overriding things there 😵‍💫)

How about:

  • We open a new discourse thread to discuss this again with the community, since no good solution seems to exist as of now
  • Keep overriding the show as per this PR, but only when a preference is set. Update the Readme accordingly.


Base.show(io::IO, ci::ComponentIndex) = print(io, "ComponentIndex($(ci.idx), $(ci.ax))")




# Show ComponentArrays
_print_type_short(io, ca; color=:normal) = _print_type_short(io, typeof(ca); color=color)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Assuming that the approach of this PR looks good, I guess we can remove all _print_type_shorts and the Base.print_type_stacktrace that I understand is not working on recent version of julia?

_print_type_short(io, T::Type; color=:normal) = printstyled(io, T; color=color)
Expand Down