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

Update/add show functions VTK objects #61

Merged
merged 6 commits into from
Aug 26, 2024

Conversation

williamjsdavis
Copy link
Contributor

The following pull request updates and adds show methods for various VTK objects, following this issue: #60

Updated and added methods are given for the following objects:

  • VTKFile,
  • VTKDataArray,
  • PVTKFile,
  • PVDFile,
  • VTKData,
  • PVTKData, and
  • PVTKDataArray.

Where possible, I have tried to copy the style of the original show method defined for VTKFile objects.

The following subsections show a comparison of the original show output verses the proposed new show output (generated from files in the test suite).

VTKFile objects

Original output

VTKFile("./celldata_appended_binary_compressed.vtu", <XMLDocument>, "UnstructuredGrid", "1.0.0", "LittleEndian", "vtkZLibDataCompressor", <appended_data>, 4434, 3085)

Proposed new output

VTKFile("./celldata_appended_binary_compressed.vtu", <XMLDocument>, "UnstructuredGrid", v"1.0.0", "LittleEndian", "vtkZLibDataCompressor", <appended_data>, 4434, 3085)

Notes

The proposed changes are only slightly different than the current implementation. The only difference is using the repr representation for the vtk_file.version::VersionNumber.

VTKDataArray objects

Original output

VTKDataArray("element_ids")

Proposed new output

VTKDataArray{Int64, 1, ReadVTK.FormatAppended}("element_ids", 32675, <DataArray type="Int64" Name="element_ids" NumberOfComponents="1" format="appended" offset="32675"/>, <VTKFile>)

Notes

The new show method lists the parametric types associated with the object, listing the data type encoding, the size of the second dimension, and the encodingformat. It also lists the other data fields, with a shortened expression for the vtk_filefield (shortened similarly to the method defined forVTKFile`).

PVTKFile objects

Original output

PVTKFile()

Proposed new output

PVTKFile("fields.pvtr", <XMLDocument>, "PRectilinearGrid", v"1.0.0", ["fields/fields_1.vtr", "fields/fields_2.vtr", "fields/fields_3.vtr", "fields/fields_4.vtr"], <vtk_files>)

Notes

Similar format to VTKFile, but including a list representation for the field vtk_filenames::Vector{String}.

PVDFile objects:

Original output

PVDFile()

Proposed new output

PVDFile("full_simulation.pvd", "Collection", <vtk_filenames>, <directories>, <timesteps>)

Notes

Similar format to VTKFile, but many of the fields now have shortened expressions.

VTKData objects:

Original output

VTKData()

Proposed new output

VTKData(["cell_ids", "element_ids", "levels", "indicator_amr", "indicator_shock_capturing"], <data_arrays>, <VTKFile>)

Notes

Here the new method shows the array for names::Vector{String}, and only short expressions for the other fields.

PVTKData objects:

Original output

PVTKData()

Proposed new output

PVTKData(<parent_xml>, <4-element Vector{VTKData}>)

Notes

Similar to VTKData, but here the field data::Vector{VTKData} uses the summary method for the shown string.

PVTKDataArray objects:

Original output

PVTKDataArray()

Proposed new output

PVTKDataArray(<parent_xml>, <4-element Vector{VTKDataArray}>)

Notes

Similar to PVTKData, where the field data::Vector{VTKDataArray} uses the summary method for the shown string.

Reasoning and justification for changes

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

Comments

Where possible, I have tried to copy the style and code formatting of the original show method defined for VTKFile objects. This is my first attempt at changes, and I'm happy to take on feedback, thank you!

Updated show functions for the following objects: VTKFile, VTKDataArray, PVTKFile, PVDFile, VTKData, PVTKData, and PVTKDataArray.
Copy link
Member

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

Thanks! I don't know why our CI tests fail...

src/ReadVTK.jl Show resolved Hide resolved
@ranocha
Copy link
Member

ranocha commented Aug 19, 2024

I fixed the CI issues in #61

@ranocha ranocha mentioned this pull request Aug 19, 2024
@ranocha
Copy link
Member

ranocha commented Aug 19, 2024

Could you please run the formatter?

import Pkg; Pkg.activate(temp = true); Pkg.add(name = "JuliaFormatter", version = v"1.0.45"); using JuliaFormatter; format(".")

@williamjsdavis
Copy link
Contributor Author

@ranocha I've added the test you requested and ran the formatter, let me know if you think it needs anything else!

Copy link
Member

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

Thanks 🙂

@ranocha ranocha merged commit 597dabd into JuliaVTK:main Aug 26, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show method strings for many objects are overly terse
2 participants