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

added facility to perform translational extrusion of structured meshes #85

Closed
wants to merge 89 commits into from

Conversation

SamMallinson
Copy link

@SamMallinson SamMallinson commented Jul 10, 2017

minor change to code for geometry.py. Fixes #83.

@nschloe
Copy link
Owner

nschloe commented Jul 11, 2017

I'd be awesome to get a simple test for that, too, so I don't accidentally mess it up in the future. For this, you basically only need one file in test/examples/ with a generate() function that returns the geometry and the expected area/volume of the mesh.

@nschloe
Copy link
Owner

nschloe commented Jul 11, 2017

Oh, and you can silence the pylint error too-many-branches in .pylintrc.

@SamMallinson
Copy link
Author

Hi Nico,
I've added an example called bend.py in test/examples. This creates a bend.geo file which is then converted into a mesh using gmsh. See the file bend.README for details of the commands.
Cheers,
Sam

@SamMallinson
Copy link
Author

Oh, and you can silence the pylint error too-many-branches in .pylintrc.

I tried adding the line too-many-branches to .pylintrc, but it still comes back with the error:

~/src/pygmsh$ pylint pygmsh
************* Module pygmsh.geometry
R:309, 4: Too many branches (13/12) (too-many-branches)


Your code has been rated at 9.99/10 (previous run: 9.99/10, +0.00)

Am I doing something wrong?

@nschloe
Copy link
Owner

nschloe commented Aug 16, 2017

Adding it to .pylintrc should do it. You could push your changes so I can see what's going wrong.

Alternatively, you could add the comment

# pylint: disable=too-many-branches

to where there are "too many branches" to only disable the warning locally.

@MacZel
Copy link

MacZel commented Dec 4, 2017

what is the current status for this issue, is it planned to be resolved in the near future?

@SamMallinson
Copy link
Author

Sorry for delay, have added disabling command as per Nico's suggestion of Aug 16, pylint now runs fine. Changes committed and pushed.

@MacZel
Copy link

MacZel commented Dec 4, 2017

Thanks, the change is essential for my master thesis :)
For now I'm using local build

@gdmcbain
Copy link
Contributor

gdmcbain commented Dec 5, 2017

I wrote an example, SamMallinson#1, simpler than the ‘malfunctioning bend’ retracted in f200657. It's based on test/test_screw.py, but without the rotation.

It demonstates the proposed layers keyword argument for the .extrude method.

It currently writes .msh rather than .vtu, as I don't have VTK installed. (Quite difficult for Python 3.6, and I don't need it for anything else.) Feel free to change back to vtu.

The original example wrote a .geo file in order to specify Surface Recombine and Mesh.RecombineAll, which are needed to get quadrilaterals on the primary surface and hexahedra through the volume. These are the most obvious application of the new functionality but not strictly needed; the current example just makes tetrahedra (in one layer). (Actually, I wasn't sure how to specify those options without writing a .geo file, which none of the other examples do.)

new file to test extrude(..., layers=1)
gdmcbain added a commit to gdmcbain/pygmsh that referenced this pull request Dec 5, 2017
@gdmcbain
Copy link
Contributor

gdmcbain commented Dec 5, 2017

Oops, didn't realize that numpy was only needed in the rotational part of test/test_screw.py; deleted unneeded import in SamMallinson#2

@nschloe
Copy link
Owner

nschloe commented Dec 7, 2017

The diff of this PR is huge. Not sure what went wrong exactly, but it might be better just moving the changes to a new, clean branch.

@gdmcbain
Copy link
Contributor

gdmcbain commented Dec 7, 2017

Sorry, I think it's mostly merging things from nschloe/master added since 10 July, looking at

master...gdmcbain:translationalExtrude

but I'll start afresh to make it easier to assess.

@nschloe
Copy link
Owner

nschloe commented Dec 8, 2017

Closing in favor of PR #109.

@nschloe nschloe closed this Dec 8, 2017
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.

None yet

5 participants