todofixthis / class-registry

Registry pattern for Python classes, with setuptools entry points integration!
MIT License
40 stars 8 forks source link

AutoRegister no longer needs to return a metaclass #14

Closed todofixthis closed 10 months ago

todofixthis commented 11 months ago

Thanks to https://peps.python.org/pep-0487/ we now have __init_subclass__ which can (amongst other things) be used to automatically add classes to a registry. As a result, AutoRegister no longer needs to return a metaclass, which would substantially improve its interoperability with other libraries (e.g., Pydantic).

I think something more-or-less like this should work nicely:

def AutoRegister(registry: ClassRegistry):
    class _Base(ABC):
        def __init_subclass__(cls, **kwargs):
            super().__init_subclass__(**kwargs)
            if not isabstract(cls):
                registry.register(cls)
    return _Base

Also, I think AutoRegister can drop its base_type parameter, as developers could achieve the same outcome with multiple inheritance, e.g.:

class IDToken(BaseModel, AutoRegister(registry)):
    ...

An alternative would be to make registry a kwarg for __init_subclass__, so that AutoRegister becomes an actual class instead of a function that returns same...but that feels brittle to me, as it could potentially cause conflicts if another base class is expecting a kwarg with the same name.

class AutoRegister(ABC):
    def __init_subclass__(cls, /, registry, **kwargs):
        ...

class IDToken(BaseModel, AutoRegister, registry=registry):
    ...

But then, I've never been comfortable with this style, so maybe I'm the problem ๐Ÿ˜…

todofixthis commented 10 months ago

Need to think about how to deprecate the existing AutoRegister gracefully.

Maybe keep the existing AutoRegister but add a DeprecationWarning when it's called. Then add a new AutoRegister to class_registry.base maybe?

After a few releases I can then remove the old AutoRegister, or maybe just keep it but rename to AutoRegisterMeta? This would be a good use case for adding telemetry, but that's against my religion, so.... ๐Ÿ˜…

todofixthis commented 10 months ago

Maybe do a quick survey of public projects that use phx-class-registry in GitHub, GitLab, Bitbucket and see if anyone is using AutoRegister ๐Ÿค”

I'm not sure if I'm hoping that people are using it, or that people aren't using it ๐Ÿ˜น