nschloe / pygmsh

:spider_web: Gmsh for Python
GNU General Public License v3.0
859 stars 162 forks source link

Geometry() unusable outside `with` block #542

Open rmsc opened 2 years ago

rmsc commented 2 years ago

As far as I can tell, all tests and examples use the following structure:

with pygmsh.geo.Geometry() as geom:
    # Do stuff

In my case I want to store the geometry inside a class, like so:

class MyClass:
    def __init__(self):
        self.geom =  pygmsh.geo.Geometry()

Common sense says this should work just as fine, but it doesn't. I just get an error on each operation:

Error   : Gmsh has not been initialized

Looking at the source code, in src/pygmsh/common/geometry.py it is clear that some crucial gmsh operations are done in __enter__() and __exit__().

I could manually call gmsh.initialize() and such, but it seems awful for an abstraction API. I could also call __enter__() and __exit__() manually, but that's even uglier.

A good compromise might be to just move the enter/exit code to proper initialize() and finalize() methods, calling them from __enter()__ and __exit()__.

As an alternative, the enter/exit code could perhaps be moved to the constructor/destructor.

danshapero commented 2 years ago

I could also use this; it looks like the main concern is what assumptions gmsh makes internally. The API really seems like it wants you to work on only one model at a time and using a context manager helps to guarantee that. If multiple initialization / destruction is a concern, gmsh has a function is_initialized that could help avoid anything too awful from happening, but there might be other concerns as well.

I'd be happy to take a crack at this but it would be good to get some feedback from @nschloe first.