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

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

Merged
merged 5 commits into from
Mar 23, 2020

Conversation

gwater
Copy link
Contributor

@gwater gwater commented Nov 26, 2019

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
Copy link

codecov bot commented Nov 26, 2019

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%> (ø) ⬆️
src/io/msh.jl 83.33% <83.33%> (ø)
src/io/2dm.jl 92% <0%> (+12.68%) ⬆️
src/io/obj.jl 82.6% <0%> (+17.09%) ⬆️
src/io/ply.jl 100% <0%> (+26.15%) ⬆️
src/io/stl.jl 98.33% <0%> (+34.2%) ⬆️
src/io/off.jl 100% <0%> (+43.39%) ⬆️

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
Copy link
Contributor Author

gwater commented Nov 26, 2019

relevant to #11

@sjkelly
Copy link
Member

sjkelly commented Dec 22, 2019

@gwater Is this good to merge?

@gwater
Copy link
Contributor Author

gwater commented Dec 23, 2019

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
Copy link
Contributor Author

gwater commented Dec 23, 2019

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
Copy link
Contributor Author

gwater commented Mar 13, 2020

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 added a commit to JuliaIO/FileIO.jl that referenced this pull request Mar 13, 2020
Needs this to be merged & tagged first: JuliaIO/MeshIO.jl#52
@SimonDanisch
Copy link
Member

I removed the init and created a proper registry entry in FileIO: JuliaIO/FileIO.jl#258

@gwater
Copy link
Contributor Author

gwater commented Mar 13, 2020

thanks!

@gwater
Copy link
Contributor Author

gwater commented Mar 18, 2020

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

@gwater
Copy link
Contributor Author

gwater commented Mar 18, 2020

simplified tests a little and triggered retesting

@gwater
Copy link
Contributor Author

gwater commented Mar 18, 2020

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

@gwater
Copy link
Contributor Author

gwater commented Mar 18, 2020

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

$ julia13 MeshIO/test/runtests.jl 
Test Summary: |  Pass  Total
MeshIO        | 95360  95360

@gwater
Copy link
Contributor Author

gwater commented Mar 23, 2020

any chance this can move ahead?

@SimonDanisch
Copy link
Member

Thank you :)

@SimonDanisch SimonDanisch merged commit 321115d into JuliaIO:master Mar 23, 2020
@gwater
Copy link
Contributor Author

gwater commented Mar 23, 2020

thanks!

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.

3 participants