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

Document difference between Base.close and vtk_save #148

Closed
KnutAM opened this issue Sep 18, 2024 · 3 comments · Fixed by #149
Closed

Document difference between Base.close and vtk_save #148

KnutAM opened this issue Sep 18, 2024 · 3 comments · Fixed by #149

Comments

@KnutAM
Copy link

KnutAM commented Sep 18, 2024

In Ferrite.jl (Ferrite-FEM/Ferrite.jl#1055) we noticed that close(::WriteVTK.CollectionFile) is defined (on the supertype VTKFile), but doesn't actually cause the file to be saved which requires vtk_save.

What is the intended difference? (I couldn't understand the difference based on the docs, hence the issue to document this)

I'm also curious why close cannot always be used and why vtk_save is required?

@jipolanco
Copy link
Member

Good question. Currently, close simply deallocates the memory associated to the construction of an XML file by the LightXML library. It has the same definition for all kinds of VTK XML files (VTKFile). In its current form, this function should be considered as internal and a user should never call it directly – despite the fact that it's actually documented, and the documentation is in fact incorrect and should be fixed...

In contrast, vtk_save actually saves a VTK file, while also calling close internally to deallocate LightXML objects.

In short, currently, a user should only call vtk_save and never call close. But it would make a lot of sense to change the definition of close to make it equivalent to vtk_save.

Note that none of this matters when using the do-block syntax as recommended in the docs (the file is automatically saved at the end), but I agree that the syntax can be inconvenient in some cases.

@fredrikekre
Copy link
Contributor

this function should be considered as internal and a user should never call it directly

In a sense it becomes public when extending Base.close so that would be a good first thing to change.

@jipolanco
Copy link
Member

I agree. At least one would expect close to actually do what its docstring states:

Write and close VTK file.

So the simplest solution is to redefine it to do just that and keep it a public function.

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 a pull request may close this issue.

3 participants