6
\$\begingroup\$

I am creating a metaclass which ensures that instances of an actual class A are kind of singletons. But rather than having only single instance of A, I would like to have one instance per argument set given to A.

That is

class A(metaclass=ArgumentSingleton):
    pass

r = A()
s = A()
t = A(1)
u = A(1)
v = A('hi')

print(r is s)  # True
print(r is t)  # False
print(t is u)  # True
print(v is u)  # False
print(v is r)  # False

Moreover, the property of 'being a singleton' must survive pickling.

That is

a = A()

with open('tmp.pkl', 'wb') as f:
    pkl.dump(a, f)

with open('tmp.pkl', 'rb') as f:
    b = pkl.load(f)

print(a is b)  # True

For example Sympy is capable of doing that with their constants (such as pi) and I tried similar approach.

class ArgumentSingleton(type):
    _instances = {}

    def __new__(cls, name, bases, class_dict):
        def __init__(self, *args):
            self._args = args
            print('Here in __init__')

        def __reduce_ex__(self, protocol):
            return type(self), (*self._args,)

        init = '__init__'
        reduce = '__reduce_ex__'
        method_absence = [
            init not in class_dict,
            reduce not in class_dict
        ]

        if all(method_absence):
            class_dict[init] = __init__
            class_dict[reduce] = __reduce_ex__
        elif any(method_absence):
            raise ArgumentSingletonException(
                f"Either both methods '{init}', '{reduce}' are defined or "
                f"none of them in class: {name}"
            )

        new_class = super().__new__(cls, name, bases, class_dict)

        return new_class

    def __call__(cls, *args, **kwargs):
        key = (cls, *args, *kwargs.items())
        if key not in ArgumentSingleton._instances:
            ArgumentSingleton._instances[key] = super().__call__(*args, **kwargs)
        return cls._instances[key]

The code depends quite deeply (at least to my standards) on inner workings of Python and I am afraid of any hidden bugs and hidden problems.

The purpose of controling presence of __init__ and __reduce_ex__ is to explicitly move responsibility for __reduce_ex__ to the creators of the class A in case they decide to provide their __init__.

Any comments or suggestions appreciated!

So far, I am aware of the fact that the arguments must be hashable. That is not problem for me, as this construction is meant to speed up comparison of my complex hashable objects.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Hey, welcome to the community! The 2nd code snippet, where you show the pickling and unpickling, makes use of a variable b that is not defined! For the sake of clarity and correctness you might want to add it there :) \$\endgroup\$ – RGS Apr 21 at 12:19
  • 1
    \$\begingroup\$ Hi, thank you for welcome and pointing out the error. Fixed. \$\endgroup\$ – Tomáš Hons Apr 21 at 12:21
  • 1
    \$\begingroup\$ Welcome to Code Review. I have rolled back your edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Heslacher Apr 21 at 14:34
5
\$\begingroup\$

The point of having to define either neither or both of __init__ and __reduce_ex__ is to set _args. However you can just set _args in __call__ and then use the one you've set in your __reduce_ex__.
Note: untested

class ArgumentSingleton(type):
    __INSTANCES = {}

    def __new__(cls, name, bases, class_dict):
        def __reduce_ex__(self, protocol):
            return partial(type(self), *self.__args, **self.__kwargs), ()

        def __init__(self, *args, **kwargs):
            pass

        class_dict.setdefault('__init__', __init__)
        class_dict.setdefault('__reduce_ex__', __reduce_ex__)
        return super().__new__(cls, name, bases, class_dict)

    def __call__(cls, *args, **kwargs):
        key = (cls, args, frozendict(kwargs))
        if None is (self := cls.__INSTANCES.get(key)):
            self = cls.__INSTANCES[key] = super().__call__(cls, *args, **kwargs)
            self.__args, self.__kwargs = key[1:]
        return self

Now we should be able to see we can just stop defining a metaclass by;

  1. changing __call__ to __new__, and
  2. defining a default __reduce_ex__ on the base class.
from frozendict import frozendict
from functools import partial

class Singleton:
    __INSTANCES = {}

    def __new__(cls, *args, **kwargs):
        key = (cls, args, frozendict(kwargs))
        if None is (self := cls.__INSTANCES.get(key)):
            self = cls.__INSTANCES[key] = super().__new__(cls)
            self.__args, self.__kwargs = key[1:]
        return self

    def __reduce_ex__(self, protocol: int):
        return partial(type(self), *self.__args, **self.__kwargs), ()

class A(Singleton):
    pass


# passes all your tests and the following one
a = A("hi")
b = pickle.loads(pickle.dumps(a))
assert a is b
\$\endgroup\$
12
  • \$\begingroup\$ Thanks for your answer. However it gets little problematic when allowing keyword arguments. Please, see my edit. \$\endgroup\$ – Tomáš Hons Apr 21 at 14:21
  • 1
    \$\begingroup\$ @TomášHons My aim is to simplify your code not change how your code functions. Since your code didn't correctly handle kwargs my changes don't. \$\endgroup\$ – Peilonrayz Apr 21 at 14:26
  • \$\begingroup\$ Well, I my solution explicitly forbid them, which I see as better than allow them and silently introduce weird behavior. I pointed that out for anyone who wants to use the solution (me and you included). My intention was not to complain but to raise awareness of the possible problem. Your solution has +1 from me, as it helped me a lot and probably will be accepted (I want let the question open for others for some time). \$\endgroup\$ – Tomáš Hons Apr 21 at 14:39
  • \$\begingroup\$ @TomášHons Yeah using frozendict would be better here as you know. Additionally you are unlikely to have weird gotchas. For example if a subclass change the __init__ in your code to include **kwargs your code has the same issue, without changing to use frozendict. \$\endgroup\$ – Peilonrayz Apr 21 at 14:43
  • \$\begingroup\$ Right, that is true, it's not bullet proof (maybe forbid __init__ completely). Similarly, I still ponder whether use the Singleton should be a class or metaclass. With metaclass it is possible to avoid invoking __init__ completely when re-using an instance which is probably desirable. \$\endgroup\$ – Tomáš Hons Apr 21 at 15:22
0
\$\begingroup\$

Here is the solution that I will probably use.

It's a bit more complicated, but

  • Enables the class A to define its own signature of __init__ (without introducing new problems).
  • The signature is correctly handled even when it uses default arguments.
  • The __init__ method is truly invoked only once (which appears to me as more logical behavior).

The first two properties are ensured via inspect module and manipulating with the actual signature of A.__init__. The second property comes from the use of metaclass and modifying behavior of its __call__ method. Modifying __new__ in the superclass of A is not enough for this purpose.

from frozendict import frozendict
from functools import partial
import inspect

class ArgumentSingletonMeta(type):
    __instances = {}

    @staticmethod
    def __get_trimmed_signature(init_method):
        # Removes 'self' argument from the signature
        sig = inspect.signature(init_method)
        params = tuple(sig.parameters.values())
        return sig.replace(parameters=params[1:])

    @staticmethod
    def __get_normalized_arguments(cls, *args, **kwargs):
        sig = ArgumentSingletonMeta.__get_trimmed_signature(cls.__init__)
        bound = sig.bind(*args, **kwargs)
        bound.apply_defaults()
        return bound

    def __call__(cls, *args, **kwargs):
        normalized = ArgumentSingletonMeta.\
            __get_normalized_arguments(cls, *args, **kwargs)
        key = (cls, normalized.args, frozendict(normalized.kwargs))
        if None is (self := cls.__instances.get(key)):
            self = cls.__instances[key] = super().__call__(*args, **kwargs)
            self._args, self._kwargs = key[1:]
        return self

class ArgumentSingleton(metaclass=ArgumentSingletonMeta):
    def __init__(self, *args, **kwargs):
        super().__init__()
        # self._args = args
        # self._kwargs = kwargs

    def __reduce_ex__(self, protocol: int):
        return partial(type(self), *self._args, **self._kwargs), ()

Note that it is probably more clear to assign the self._args and self._kwargs attributes in ArgumentSingleton's __init__. However, setting them in ArgumentSingletonMeta's __call__ is more efficient, as the args and kwargs are shared between the __instances dict and the actual instances.

Now try the following code:

import pickle as pkl

class A(ArgumentSingleton):
    def __init__(self, x, y, z=1):
        super().__init__(x, y, z)

a = A(x=0, y=1)
b = A(0, 1)
c = A(0, 1, z=1)

print(a is b)
print(a is c)

with open('tmp.pkl', 'wb') as f:
    pkl.dump(a, f)

with open('tmp.pkl', 'rb') as f:
    b = pkl.load(f)

print(a is b)

Please comment if you see any hidden problem (even from software engineering point of view). So far, I can't see any, but I would like to know if there are any :)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.