Add plugin system for extending SolidPython

[mraleson]

I am really enjoying solid python! I hope you consider this proposal, I think it would be really helpful to grow SolidPython.

I thought it would be really nice to have a standard way to extend SolidPython. Often it makes sense to just write a helper function, but sometimes it would be really valuable to add more fundamental functionality and make use of the function chaining. I initially did this for my own projects via inheritance, but it was difficult and didn't lend itself to breaking functionality out into reusable modules. I think a plugin system would be very convenient and open things up for third party modules.

This pull request creates a simple plugin system. A plugin would be a function (or collection of functions) that extend the built in shapes with additional chain-able functionality.

Usage example:

from solid2 import *

# create a plugin
def shrink(obj):
    return obj.scale(.5, .5, .5)

# register plugin
register(shrink)

# create geometry
geometry = cube(1).shrink().translate(1, 2, 3)

# save to scad file
geometry.save_as_scad()

I imagine in a real situation someone would create a package, the package could be installed, and in your app you could register the plugin. Something like this:

from solid2 import *
from plugins import chamfer

# register plugin
register(chamfer)

# create geometry
geometry = cube(1).chamfer().translate(1, 2, 3)

# save to scad file
geometry.save_as_scad()

[jeff-dh]

A couple of thoughts about this:

• solidpythons(v2) access style syntax used to be very similar to what you're proposing up until a change ~a year ago (compare: https://github.com/jeff-dh/SolidPython/blob/d178e62c7c77c2703c54ef44adc62836bc7364f0/solid2/extensions/access_syntax.py)
• the reason why this was changed is, because the lsps can't handle dynamic classes and with the new solution we achieved full pyright support
• since "than" I'm wondering whether there's any way to extend ObjectBase with "stuff" from an extension but couldn't come up with anything static that's still dynamic enough for a "plugin". Any suggestions are welcome. I assume your solution does not work well with lsps (that's not an exclusion criterion!)
• there is already a "extension infrastructure" (compare: https://github.com/jeff-dh/SolidPython/blob/master-2.0.0-beta-dev/solid2/examples/08-extensions.py) even though it does not cover you're request

You're solution provides a different way for what I use to do like this:

#myPlugin.py
from solid2 import *
def upLeft(x):
    return x.up(1).left(1)

ObjectBase.upLeft = upLeft

>>> from solid2 import *
>>> import myPlugin
>>> cube().upLeft()
translate(v = [-1, 0, 0]) {
        translate(v = [0, 0, 1]) {
                cube();
        }
}

Intuitively I would prefer a solution like this:

#solid2/core/extension_manager.py
[...]
def register_access_syntax_extension(exts):
    if not isinstance(exts, Iterable):
        exts = [exts]

    from .object_base import ObjectBase
    for fn in exts:
        if hasattr(ObjectBase, fn.__name__):
            raise Exception(f'Unable to register "{fn.__name__}" plugin, it would overwrite an existing plugin or method.')
        setattr(ObjectBase, fn.__name__, fn)

#solid2/core/__init__.py
from .extension_manager import register_access_syntax_extension

>>> from solid2 import *
>>> def leftUp(x):
...     return x.left(1).up(1)
... 
>>> register_access_syntax_extension(leftUp)
>>> cube().leftUp()
translate(v = [0, 0, 1]) {
        translate(v = [-1, 0, 0]) {
                cube();
        }
}

Do you have any thoughts about my thoughts? ;)

[mraleson]

Thank you for reviewing the pull request! I read through all of your suggestions. Here are my thoughts:

• Regarding the change from registering an extension or plugin using a dict or a list, I also went back and forth on this. The reason I ended up choosing a dictionary and not always inferring the extension name from __name__ is because I thought there may be a case where you had two extensions you wanted to use that had clashing names or perhaps someone wanted to use a lambda or something. However, I agree using a list and always the implicit function __name__ has its advantages. I think either way is great.

def register_access_syntax_extension(exts):
    if not isinstance(exts, Iterable):
        exts = [exts]

• For setattr vs __getattr__. I had originally used setattr as well, however I thought it was better to separate the plugin/exertions instead of dynamically altering the class. I had two reasons, one is that although you may have a name clash due to inheritance, you would never be dynamically altering the class. This would make it much easier to debug identify which plugins/extensions had been installed because its not implicit on the class. Second, this could also allow people to remove or disable plugins without reloading the module etc. For those reasons I think its better to not use setattr but I am not strongly opinionated.

if hasattr(ObjectBase, fn.__name__):
    raise Exception(f'Unable to register "{fn.__name__}" plugin, it would overwrite an existing plugin or method.')
setattr(ObjectBase, fn.__name__, fn)

• Existing extension syntax vs plugin etc. I was hopeful that naming it plugin might differentiate it from the existing extension system while keeping it simple. I prefer a simpler name, however I think register_access_syntax_extension is also great.

• Regarding LSPs, I don't use an IDE so I don't have a strong opinion there. I did quickly download pyright and check against my pull request and example code. It didn't seem to complain, but I haven't used pyright specifically before, so I'm not sure. Maybe pyright is okay with __getattr__?

Those are my thoughts in detail, however I am completely okay with all of your changes. I think ability to easily extend the access syntax with a plugin or extension system is really valuable and all that really matters to me. A change like this is an advantage of SolidPython over the default OpenSCAD language and I think it would enable people to easily extend SolidPython without having to incorporate those extensions into the core code.

[jeff-dh]

The solution proposed in the mentioned branch implements your solution but slightly different integrated. I would suggest that solution.It keeps as much as possible out of the core and at the places they are supposed to belong. Furthermore it also complies with your thoughts mentioned, if I understood everything correct "on the fly".

I'd love to see this -- the plugin extension space -- of solidpython growing! There is already a repository (and package) where I'd like to gather plugins. But the repo and package is atm actually a little mess, but we can clean it up.

https://github.com/jeff-dh/solidpython2_extensions/tree/master/solid2_extensions https://pypi.org/project/solidpython2-extensions/

[jeff-dh]

It does not comply with all your ideas ;)

I also thought about having different plugins / extensions and namespace clashes.... I don't have an opinion about it yet. All proposed solution so far have major drawbacks for me. I like that each extension registers itself and you only have to import and use it. I don't know yet how to solve the namespace clashes yet. Your proposal reduces the issue, but I think it can still become tricky. Especially if you're using 3rd party libraries which use extensions itself.....

Syntax: I totally agree, but solidpython is already extendible with some stuff (render extensions, root wrappers, fonts). I think it makes sense to have one central infrastructure that handles them all instead of several different approaches to more or less the same thing. You would end up having different extensions for different stuff instead of having one extension which might contain several single extensions for different "subsystems". Therefor "register" is to general. I also though about renaming "default_extension_manager" to something shorter. Maybe I'll come up with something. Or maybe I'll add some global wrapper functions. Again, I'll think about it....

List vs Dict: The branch mentioned does not allow to add lists or dicts at all. One needs to register every single extensions by it's own. The reason is that my design proposal keeps the register-book-keepig inside the plugins and therefor I think is feasible to register every one on it's own.

[mraleson]

Awesome, thank you. That all makes sense to me. I reverted my commit and brought your alternative implementation into this pull request.

[jeff-dh]

Regarding namespace clashes I would suggest the following:

Every extensions could prepend it's (access syntax) commands with something -> mylib_chamfer or ml_chamfer or similiar. This can be done by simply naming the functions according to it. Furthermore we could add a "prepend" parameter or "name" parameter to register_access_syntax if necessary.

I think this might be good enough for the scope of SolidPython and if it turns out that it is not, we can adjust according to emerging needs.

I'm wondering whether it is possible to move the extensions register stuff into function decorators:

@solid2.register_access_syntax("some_alternative_name")
def leftUp(x):
    return x.left(1).up(1)

@solid2.register_pre_render
def pre_render_extensions(...?):
    [...]

or similar.....

[jeff-dh]

Haha, yeah it should work:

The docs say:

Decorator expressions are evaluated when the function is defined, in the scope that contains the function definition. 

and the repl says:

>>> def foo(fn):
...     print("foo")
...     return fn
... 
>>> @foo
... def blub():
...     print("blub")
... 
foo
>>> blub()
blub
>>> @foo
... class bla:
...     pass
... 
foo

So I assume the proposed decorators should be straight forward.

If we find "good" names for the function decorators this would also solve the bad naming (-> default_extensions_manager.register_access_syntax) issue we talked about earlier.

If you have better suggestions for the decorator names, let me know!

[jeff-dh]

(I can't push to your branch for some reason I don't understand.... it gets rejected ?!?)

Have a look at this.

[jeff-dh]

note: my accessSyntaxExtension branch turned into a dev branch which already contains a couple other changes. If you like you can merge it completely into the branch of this pr, otherwise I will merge it directly into master at some point.

[mraleson]

Closed because #38 now includes these changes