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

weak assumptions on class methods #170

Open
Borda opened this issue Feb 13, 2024 · 4 comments
Open

weak assumptions on class methods #170

Borda opened this issue Feb 13, 2024 · 4 comments
Labels

Comments

@Borda
Copy link
Contributor

Borda commented Feb 13, 2024

In the readme, it is set that a class method shall not be cased if they use self, but nothing prevents top so, and the user can easily fall to wrong values; I think that we shall allow ONLY static methods and raise exceptions on anything else without making make many assumptions if the code is safe...

import cachier


class DummyClass():
    def __init__(self, state=0):
        self.state = state

    @cachier.cachier()
    def add(self, a):
        return self.state + a


object_1 = DummyClass(1)
print(object_1.add(2))
object_2 = DummyClass(2)
print(object_2.add(3))
@Borda Borda added the bug label Feb 13, 2024
@Borda
Copy link
Contributor Author

Borda commented Feb 15, 2024

Here is the code for detecting static:

def is_function_static(func):
    for obj in globals().values():
        if inspect.isclass(obj) and hasattr(obj, func.__name__) and func.__name__ in obj.__dict__:
            method = obj.__dict__[func.__name__]
            if method is not None and isinstance(method, staticmethod):
                return True
    return False

@shaypal5
Copy link
Collaborator

OK. Finally read that.

I tend to agree.

Every useful functionality of the current implementation with methods can be used by writing a static helper function and caching it using cachier instead. Thus, I would feel comfortable with blocking this use, with a note of this proper related use.

But is the same true for class methods? Help me think. Let's try to challenge our selves here.

@Borda
Copy link
Contributor Author

Borda commented Feb 19, 2024

well, you can set by default some global config with strict validation, and if a user wants he would allow weak checks..

@shaypal5
Copy link
Collaborator

Ok. Agreed. So it can be on by default, and you can opt-out with a config.

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

No branches or pull requests

2 participants