Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is my in-class decorator not Pythonic enough or PyCharm not smart enough in lint warning?

I want to define a decorator within a class. I don't want to define it as a separated, independent function, for this decorator is specifically for this class and I want to keep the correlated methods together.

The purpose of this decorator is to check some prerequisites, especially the database connection, SSH connection, etc., which are held by member variables, are still available. If not, the decorated function won't be called and some error reporting and clean-up works will be done.

I made the following testing class to test if it works, and the code did work well. But I found that PyCharm shows warnings to this piece of code. So I wonder, if it means that my code is not Pythonic, or PyCharm is not smart enough and gave this warning by mistake?

If my code is is not Pythonic, how to change? If it is PyCharm's mistake, how shall I and my team configure PyCharm to let it specifically ignore this kind of warning while keep most other lint check?

class TestClass:
    def __init__(self):
        self.flag = True

    def dec(func):
        def wrapper(self, *args, **kwargs):
            if not self.flag:
                print("Won't run!")
                return empty_fun(self, *args, **kwargs)
            return func(self, *args, **kwargs)

        def empty_fun(*args, **kwargs):
            return None

        return wrapper

    @dec
    def foo(self):
        print("foo")

    @dec
    def bar(self, msg, more, *args, **kwargs):
        print("message: %s" % msg)
        print("more %s:" % more)
        for item in args:
            print("other item: %s" % item)
        name = kwargs.get('name')
        age = kwargs.get('age')
        print('name: %s' % name)
        print('age: %s' % age)


def main():
    t = TestClass()
    t.foo()
    print('-'*10)
    t.bar("abc", 'def', 'hij', 'klm', name='Tom', age=20)


if __name__ == '__main__':
    main()

Here is the lint warning reported by PyCharm:

Lint warning reported by PyCharm

like image 591
Vespene Gas Avatar asked Jul 10 '18 08:07

Vespene Gas


2 Answers

Your code is technically correct (in that it will work as expected), with the caveat that dec will become a method of TestClass and will break if invoked as such. You should at least make it a staticmethod to avoid this.

wrt/ pythonicity, it IS indeed unpythonic to make this decorator part of the class when it's not needed. The fact it only applies to this class is not a reason to make it a member of the class, and even less to make it part of the public API.

You can probably add linter hints in comments to silence it, but I'd personnally just extract this decorator from the class, make it private, and document that it's only supposed to be used with this class.

As as side note: I assume your empty_func is a placeholder for the "error reporting and clean-up work" - else it's just plain useless -, but does it really need to be defined in the decorator?

like image 178
bruno desthuilliers Avatar answered Nov 07 '22 05:11

bruno desthuilliers


As of Python decorator as a staticmethod, it seems that putting a decorator inside a class is discouraged.

Come to think of it, Python provides this nice compartmentalization called modules. Have you considered putting all code related to this class inside a module, and the other code in other modules?

I'd strongly recommend to move the decorator to the module scope -- it does not seem to belong inside the class. If you want to keep it inside the class, don't make it a staticmethod, but rather simply del it at the end of the class body -- it's not meant to be used from outside the class in this case.

like image 44
serv-inc Avatar answered Nov 07 '22 05:11

serv-inc