JuliaIO / MeshIO.jl

IO for Meshes
Other
79 stars 31 forks source link

Implement basic parser for triangular meshes from .MSH version 4 (GMSH) #52

Closed gwater closed 4 years ago

gwater commented 4 years ago

I partially implemented a parser for .MSH files (version 4) based on the current GMSH manual. Its not complete; especially missing is any support for non-Triangle elements (lines, hedrons, points, etc). Also, only the text version of the format is supported.

codecov[bot] commented 4 years ago

Codecov Report

Merging #52 into master will increase coverage by 24.98%. The diff coverage is 83.82%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #52       +/-   ##
===========================================
+ Coverage   67.21%   92.19%   +24.98%     
===========================================
  Files           6        7        +1     
  Lines         302      282       -20     
===========================================
+ Hits          203      260       +57     
+ Misses         99       22       -77
Impacted Files Coverage Δ
src/MeshIO.jl 100% <100%> (ø) :arrow_up:
src/io/msh.jl 83.33% <83.33%> (ø)
src/io/2dm.jl 92% <0%> (+12.68%) :arrow_up:
src/io/obj.jl 82.6% <0%> (+17.09%) :arrow_up:
src/io/ply.jl 100% <0%> (+26.15%) :arrow_up:
src/io/stl.jl 98.33% <0%> (+34.2%) :arrow_up:
src/io/off.jl 100% <0%> (+43.39%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 69b5851...3c08e40. Read the comment docs.

gwater commented 4 years ago

relevant to #11

sjkelly commented 4 years ago

@gwater Is this good to merge?

gwater commented 4 years ago

Yes, it doesn't implement the complete feature set of the file format but it can handle the unimplemented cases (giving warnings) and successfully loads triangular meshes (which covers my personal use case). I'm not planning to implement additional features in this PR.

gwater commented 4 years ago

you should take a look at the registration method though - as I understand this is typically done in FileIO, not through the__init__() mechanism

gwater commented 4 years ago

I added a test case to keep coverage up.

I still don't know how to properly do the file type registration – it appear two PRs need to be applied simultaneously to FileIO and MeshIO? Guidance would be appreciated

SimonDanisch commented 4 years ago

I removed the init and created a proper registry entry in FileIO: https://github.com/JuliaIO/FileIO.jl/pull/258

gwater commented 4 years ago

thanks!

gwater commented 4 years ago

as far as i can tell, tests ran against an outdated version of FileIO.jl

gwater commented 4 years ago

simplified tests a little and triggered retesting

gwater commented 4 years ago

ok i guess this wont work until the other PR is merged and that PR is waiting for this one 😅

gwater commented 4 years ago

Ok, can confirm tests pass locally when both PRs are combined:

$ julia13 MeshIO/test/runtests.jl 
Test Summary: |  Pass  Total
MeshIO        | 95360  95360
gwater commented 4 years ago

any chance this can move ahead?

SimonDanisch commented 4 years ago

Thank you :)

gwater commented 4 years ago

thanks!