Effectory?

[teutoburg]

Just a silly little post-midnight idea I just had, writing it down so I don't forget:

The way we create effects in effects.effect_utils.make_effect(), and by extent in the (rather convoluted) constructor of optics.optical_element.OpticalElement is a bit ... meh.

Maybe this could be streamlined via a factory-pattern for effects, hence the name of this issue, the effect-factory or Effactory. This would also help in the recent struggle related to the whole currsys/cmds thing, that needs to be passed to an effect. In summary, we might want to only create effects (i.e. instances of subclasses of effects.Effect) via this method. I'm not suggesting to make it impossible to directly instantiate an effect subclass, but the effectory should be the default way, that we use throughout the production code (in ScopeSim and elsewhere (IRDB etc.)).

[hugobuddel]

My 2cnts, which perhaps aren't worth much because I haven't looked into the instantiation of Effects much at all.

I feel that factory functions are not really Pythonic. To me it seems that what make_effect() does is what __new__() of the Effect class should do. Like you did with the badges.

But maybe I don't understand; what difference is there between make_effect() and a factory function (and __new__())? To me make_effect() looks exactly like what I understand a factory function should do.

Nevertheless, your main point is that the inside of these functions is a mess, and that you know a better way of doing that, which is great!

Personally, I don't like the .meta attribute. To me it seems that should all be normal class attributes. To the extent even that I think most Effect classes could be dataclasses.

[teutoburg]

Taking another look (at a more reasonable time of day), make_effect() could be described as a factory function, albeit a somewhat kludgy one. I've previously implemented two different kinds of factory in Python:

1. The one you mentioned with the Badges, which is (in my opinion) rather elegant, and mimics the behavior found e.g. in the standard library's pathlib.Path (which, if instantiated, will under the hood create and return an instance of PosixPath or WindowsPath, depending on the OS. However, the downside of this is that it's not super easy to add new subclasses which are not found in scopesim.effects. There is apparently some need for this, as the comment in make_effect() reads: # ..todo: add looking for custom effect class names from 3rd party packages.
2. Something I did in a private repo a few months back (excerpt below): A separate factory class that has register and create methods. An instance of that class is created in the same module, and the subclasses are registered in a dict. This instance is then imported elsewhere and used to create the subclass instances. This approach has the advantage that new (custom) types can rather easily be registered alongside the existing ones. In our case, we'd probably want some kind of default to the scopesim.effects subpackage, as is done in make_effect(), or we do that via dynamic imports or something...

class EventFactory():
    """Generalized factory class to produce events from XML tags."""
    def __init__(self):
        self._creators = {}

    def register_type(self, tag: str, creator: Callable) -> None:
        """Add tag and event class to registry."""
        self._creators[tag] = creator

    def create_event(self, tag: str, **kwargs):
        """Create event class from registered tag."""
        creator = self._creators.get(tag)
        if not creator:
            raise ValueError(tag)
        return creator(**kwargs)


factory = EventFactory()
factory.register_type("event", Event)
factory.register_type("superevent", SuperEvent)
factory.register_type("multievent", MultiEvent)
factory.register_type("subevent", SubEvent)

[hugobuddel]

Registration of subclasses is a common usecase for meta classes. Perhaps registering classes is the main reason meta classes even exist in Python.

Everything is a object in Python; classes are instances of a meta-class (which in reality is just a class, nothing really meta about it). You can specify the meta class that a specific class is an instance off. So the __init__() of the metaclass can then call the register function, and that way the registration is automatic upon creation of the class.

As a syntactic sugar for the above, you can simply overload the __init_subclass__() method in the Effect class, which is effectively overloading __init__() of the meta class. So no actual metaclass programming necessary. Then subclassing Effect will automatically register the class.

Metaclasses are real fun, and also an easy to create an unintelligible mess, so buyer beware!

[hugobuddel]

About that last point; __init_subclass__() is executed when the class itself is created. So if the registration of the class involves several round trips to a database, including the creation of tables, and you have dozens of classes that all import each other, then 'suddenly' your code can become extremely slow and brittle. Not that I know of course. 😇

[teutoburg]

I've read about metaclasses in python and the __init_subclass__() before, but never actually used either. Might be a good opportunity to do so 🙃