JuliaVTK / ReadVTK.jl

Julia package for reading VTK XML files
https://juliavtk.github.io/ReadVTK.jl
MIT License
30 stars 9 forks source link

Bugfix - reading appended_data #34

Closed boriskaus closed 1 year ago

boriskaus commented 1 year ago

This fixes a bug that occurred irregularly (with files that could be read fine by Paraview). The culprit turns out to be the following line in VTKFile(filename):

appended_data = Vector{UInt8}(rstrip(raw_file_contents[offset_begin:offset_end]))

The issue is that raw_file_contents sometimes ends in a space as in (f\x12 \n\t" which rstrip changes to (f\x12" instead of (f\x12 ". Fixed with:

appended_data = Vector{UInt8}(raw_file_contents[offset_begin:offset_end][1:end-2])

Here a ZIP-file that has the problem: Convection_p00000000.vtr.zip reproduce the mistake with:

julia> vtk_read = VTKFile("Convection_p00000000.vtr");
julia> point_data = get_point_data(vtk_read);
julia> get_data_reshaped(point_data[point_data.names[9]]);
ERROR: BoundsError: attempt to access 738759-element Vector{UInt8} at index [671681:738760]
Stacktrace:
 [1] throw_boundserror(A::Vector{UInt8}, I::Tuple{UnitRange{Int64}})
   @ Base ./abstractarray.jl:703
 [2] checkbounds
   @ ./abstractarray.jl:668 [inlined]
 [3] view
   @ ./subarray.jl:177 [inlined]
 [4] get_data(data_array::VTKDataArray{Float32, 1, ReadVTK.FormatAppended})
   @ ReadVTK ~/Downloads/ReadVTK.jl/src/ReadVTK.jl:756
 [5] #get_data_reshaped#5
   @ ~/Downloads/ReadVTK.jl/src/ReadVTK.jl:792 [inlined]
 [6] get_data_reshaped(data_array::VTKDataArray{Float32, 1, ReadVTK.FormatAppended})
   @ ReadVTK ~/Downloads/ReadVTK.jl/src/ReadVTK.jl:791
 [7] top-level scope
   @ REPL[11]:1
boriskaus commented 1 year ago

I can confirm that the suggested changes work & all tests pass fine. A test file is now uploaded above; feel free to add it to this repo or to a separate one.

sloede commented 1 year ago

A test file is now uploaded above; feel free to add it to this repo or to a separate one.

I don't see the test file. However, we have a separate repository for that, https://github.com/JuliaVTK/ReadVTK_examples. It would be great if you can create a PR to that repo to add the new test file there, then we can use it for a new test here.

boriskaus commented 1 year ago

It would be great if you can create a PR to that repo to add the new test file there, then we can use it for a new test here.

done here.

sloede commented 1 year ago

done here.

Thanks! I merged it so you can go ahead and add a test here.

boriskaus commented 1 year ago

done, have a look if you're happy

boriskaus commented 1 year ago

Thanks - can you register the new version so I can update my packages accordingly?

sloede commented 1 year ago

Yes, I already tried to do it but we first need https://github.com/JuliaRegistries/General/pull/83019 to be merged, since we haven't changed the repo URL after moving this package to the JuliaVTK org. I'll let you know as soon as v0.1.7 is registered.

sloede commented 1 year ago

The new release v0.1.7 is out. Sorry for the delay!

boriskaus commented 1 year ago

Thanks a lot!