Context managers with static methods or class methods?

Python Help
1

Hi,

In my code I am doing something like:

    @contextmanager
    @staticmethod
    def multithreading(nthreads : int):
        '''
        Multithreading should be used with care. This should be the only
        place where multithreading is allowed to be turned on.

        Parameters
        ----------------
        nthreads: Number of threads for EnableImplicitMT
        '''
        old_val = RDFGetter._allow_multithreading
        RDFGetter._allow_multithreading = True

        EnableImplicitMT(nthreads)
        try:
            yield
        finally:
            DisableImplicitMT()
            RDFGetter._allow_multithreading = old_val

and I am intending to use this with:

with RDFGetter.multithreading(nthreads=3):
      my_function()

Is this the correct approach? ChatGpt keeps telling me that I should use class methods instead of static methods.

Cheers.

2

It’s not really a static method because of:

and the other two references. In _allow_multithreading you’re setting state on the class too, shared between all instances of it. So at the bare minimum, I’d make it into a classmethod, e.g.:

class RDFGetter:
    @contextmanager
    @classmethod
    def multithreading(cls, nthreads : int):
        '''
        Multithreading should be used with care. This should be the only
        place where multithreading is allowed to be turned on.

        Parameters
        ----------------
        nthreads: Number of threads for EnableImplicitMT
        '''
        old_val = cls._allow_multithreading
        cls._allow_multithreading = True

        EnableImplicitMT(nthreads)
        try:
            yield cls
        finally:
            DisableImplicitMT()
            cls._allow_multithreading = old_val

As the docstring says, multithreading should only be called once. I’d add in a basic lock to ensure that. Storing state on the class, in RDFGetter.multithreading takes over the entire class, and breaks other instances of the same class (and maybe even itself). So in general, I think it’s much better to use an instance method and instance variables to store state, instead of class variables. But here:
RDFGetter is basically a singleton, so it’s much of a muchness as in Python everything is an object, including classes (but it could be much more canonical and robust by either designing it for instances, or at least preventing other instances with an __init__(self): that raises an Exception).

3

Hi @JamesParrott

Thanks for your reply. I think there is a problem in the code you showed, shouldn’t the context manger and class method lines be swapped?

Cheers.

4

You’re welcome. I’ve not tested it. So if that doesn’t work, yes try switching the order of the decorators. Maybe the order doesn’t matter for the staticmethod one.

5

After a long chat with ChatGpt, it seems that what I was doing was dangerous, because I was giving a descriptor to a generator function. So apparently wrapping with classmethod should be done only to actual functions and this might be a better approach:

    @classmethod
    def multithreading(cls, nthreads : int):
        '''
        Multithreading should be used with care. This should be the only
        place where multithreading is allowed to be turned on.

        Parameters
        ----------------
        nthreads: Number of threads for EnableImplicitMT. If number
        of threads is 1, multithreading will be off
        '''

        if nthreads <= 0:
            raise ValueError(f'Invalid number of threads: {nthreads}')

        if cls._allow_multithreading:
            raise ValueError(f'Multithreading was already set to {cls._nthreads}, cannot set to {nthreads}')

        @contextmanager
        def _context():
            if nthreads == 1:
                yield
                return

            old_val = cls._allow_multithreading
            old_nth = cls._nthreads

            cls._nthreads             = nthreads
            cls._allow_multithreading = True
            EnableImplicitMT(nthreads)

            try:
                yield
            finally:
                DisableImplicitMT()
                cls._allow_multithreading = old_val
                cls._nthreads             = old_nth

        return _context()

and it seems to be passing all the tests. It also has the lock you suggested, _allow_multithreading.

Cheers.

6

Great. Well done.