Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Either incorrect terminology, or a wrong implementation of B019 check #250

Closed
khrapovs opened this issue Mar 30, 2022 · 2 comments
Closed

Comments

@khrapovs
Copy link

Following up on #218.

From the main README:

B019: Use of functools.lru_cache or functools.cache on class methods can lead to memory leaks. The cache may retain instance references, preventing garbage collection.

Here I stress the term class methods. My understanding of this term is something like:

class Foo:
    def __init__(self, x):
        self.x = x

    @functools.cache
    def compute_method(self, y):
        ...

    @classmethod
    @functools.cache
    def cached_classmethod(cls, y):
      ...

Here cached_classmethod is a "class method" since it is decorated with @classmethod. compute_method, on the other hand is not a "class method". Maybe "instance method", but definitely no "class method".

So, reading the explanation of B019 I expect that B019 is raised on cached_classmethod, but not on compute_method. The current library behavior is the reverse.

Either I am very bad with conventional terminology, which is very likely, or the behavior is wrong. Would very much appreciate some clarification on this matter. Thanks!

@jpy-git
Copy link
Contributor

jpy-git commented Mar 30, 2022

Hi @khrapovs, this is a duplicate of #236 and was fixed in #237. The rule is working correctly, but as you said, the use of class methods was poorly worded and confusing with classmethods.

This fix was shipped as part of version 22.3.23 so can you update to the latest version and you should see the new error message 👍

Recommend this video if you want context behind this lint

@khrapovs
Copy link
Author

@jpy-git Thank you very much for taking your time to clarify this. Much appreciated. Since I do not have much time and desire to go deep into understanding how caching in combination with classes works, the error message was confusing to me. Now it's clear and I have to live with it. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants