FEM: export of netgen/gmsh mesh into fenics xml format

About the development of the FEM module/workbench.

Moderator: bernd

Post Reply
joha2
Posts: 303
Joined: Tue Oct 11, 2016 9:48 pm

FEM: export of netgen/gmsh mesh into fenics xml format

Post by joha2 »

One file added. Since this is my first PR to a large project I would be glad to hear comments or critics about style/code/....
Thanks, best wishes Johannes
ian.rees
Posts: 696
Joined: Sun Jun 15, 2014 3:28 am
Contact:

Re: FEM: export of netgen/gmsh mesh into fenics xml format

Post by ian.rees »

Hi Johannes, welcome! Just a few minor comments since you asked for them - none of these are big issues. If you decide to change your files, just push --force to the same branch on github where you made the PR (Pull Request), and the PR will be automatically updated. Thanks! -Ian-

1) Python indentation is a contentious issue (well, there's really only one right answer ;) ) - I notice your new file uses a mix of spaces and tabs. Best to use 4 spaces per level everywhere, next best is to use tabs everywhere. But, I think FEM specifically has a Python coding standard which will almost surely discuss indentation.

2) I think this Pull Requests forum is being phased out - probably better to put forum posts about developing in the regular "Developer's Corner" (there's certainly some stale documentation that refers to posting here instead of just to github - if you notice any it's good to fix it).

3) get_FemMeshObjectDimension() has a structure that I found confusing (and a little commented-out "dead code" using eval - best to delete that). I think this would be more clear:

Code: Select all

if fem_mesh_obj.FemMesh.Volumes != ():
    return 3
if fem_mesh_obj.FemMesh.Faces != ():
    return 2
...
That said, some people like only having one return per method. If you're one of those, then perhaps:

Code: Select all

dims = None
if fem_mesh_obj.FemMesh.Nodes != ():
    dims = 0
if fem_mesh_obj.FemMesh.Edges != ():
    dims = 1
if fem_mesh_obj.FemMesh.Faces != ():
    dims = 2
if fem_mesh_obj.FemMesh.Volumes != ():
    dims = 3
return dims
joha2
Posts: 303
Joined: Tue Oct 11, 2016 9:48 pm

Re: FEM: export of netgen/gmsh mesh into fenics xml format

Post by joha2 »

Hi Ian! Thanks for the tips and comments! I heard from bernd, that FEM development anyway goes via his fork. This PR was somewhat accidentally ;-).
To your points: 1) I searched for the refactoring button in atom and then I just forget to continue searching ;-) I will fix this, thank you.
2) As I mentioned, I will PR via bernds FEM fork. Somewhere it is written that PRs have to be mentioned in the forum.
3) Thank you for the comments on the code. To perform the check the other way around, like you suggested, is obviously more elegant! :-) Thanks for the hint!

Best wishes and good night
Johannes

Edit: fixed and PR updated
Post Reply