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

Show method strings for many objects are overly terse #60

Closed
williamjsdavis opened this issue Jul 18, 2024 · 1 comment · Fixed by #61
Closed

Show method strings for many objects are overly terse #60

williamjsdavis opened this issue Jul 18, 2024 · 1 comment · Fixed by #61
Labels
enhancement New feature or request

Comments

@williamjsdavis
Copy link
Contributor

williamjsdavis commented Jul 18, 2024

As a new user of the package, I was surprised by the REPL show method results for many objects in this package. Upon loading creating a VTKData object, the displayed string in the REPL read "VTKData()", making me think that I had incorrectly loaded my file and the object was empty. Only upon running the dump function on the object did I then see that I had, in fact, loaded the file correctly.

In this package, VTKFile objects have an informative show method:

ReadVTK.jl/src/ReadVTK.jl

Lines 169 to 180 in 09aea36

function Base.show(io::IO, vtk_file::VTKFile)
return print(io, "VTKFile(",
"\"", vtk_file.filename, "\", ",
"<XMLDocument>", ", ",
"\"", vtk_file.file_type, "\", ",
"\"", vtk_file.version, "\", ",
"\"", vtk_file.byte_order, "\", ",
"\"", vtk_file.compressor, "\", ",
"<appended_data>", ", ",
vtk_file.n_points, ", ",
vtk_file.n_cells, ")")
end

Slightly similarly, VTKDataArray objects have a show method that is slightly helpful:

ReadVTK.jl/src/ReadVTK.jl

Lines 657 to 659 in 09aea36

function Base.show(io::IO, data_array::VTKDataArray)
return print(io, "VTKDataArray(\"", data_array.name, "\")")
end

On the other hand, the following objects have REPL show methods that I believe to be too terse:

PVTKFile objects

ReadVTK.jl/src/ReadVTK.jl

Lines 278 to 280 in 09aea36

function Base.show(io::IO, vtk_file::PVTKFile)
return print(io, "PVTKFile()")
end

PVDFile objects:

ReadVTK.jl/src/ReadVTK.jl

Lines 346 to 348 in 09aea36

function Base.show(io::IO, d::PVDFile)
return print(io, "PVDFile()")
end

VTKData objects:

Base.show(io::IO, data::VTKData) = print(io, "VTKData()")

PVTKData objects:

Base.show(io::IO, data::PVTKData) = print(io, "PVTKData()")

PVTKDataArray objects:

Base.show(io::IO, vtk_file::PVTKDataArray) = print(io, "PVTKDataArray()")

Reason changes should be made

Informative show methods help new users understand the functionality of the package, and help debugging.

Suggested changes

I suggest that the show methods for the above listed objects should show short, informative summaries. I think it's open to discussion exactly what information should be shown.

@ranocha
Copy link
Member

ranocha commented Jul 25, 2024

Sounds reasonable. If you make a PR, we will be happy to review it and assist you.

@ranocha ranocha added the enhancement New feature or request label Jul 25, 2024
@ranocha ranocha linked a pull request Aug 19, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants