-
Notifications
You must be signed in to change notification settings - Fork 11
Description
Hi!
Thanks for this lib! @lru_cache behaviour is tricky when it comes to classes and it has bitten me a few times more than I'd like to admit. This lib is now in my standard toolkit :)
I recently ran into a use-case that I think is not well defined, or at least the behaviour is different from what I would expect.
See the below case
class A:
@lru_cache()
@staticmethod
def s(x):
return x
class B(A):
pass
a1 = A()
b1 = B()
a1.s(1)
a1.s(1)
b1.s(1)
assert a1.s.cache_info().hits == 1
assert b1.s.cache_info().hits == 0 # This shows that cache is not shared, where I would expect itThis evaluates successfully, but shows an issue. It turns out that if a class B is a subclass of A, the lru_cache between staticmethods is not shared. My expectation was that this would happen, as long as the method is not explicitly overwritten by the override decorator.
In [7]: from methodtools import lru_cache
...:
...:
...: class A:
...: @staticmethod
...: def s(x):
...: return x
...: @classmethod
...: def c(cls,x):
...: return x
...:
...:
...: class B(A):
...: pass
...:
In [8]: A.c
Out[8]: <bound method A.c of <class '__main__.A'>>
In [9]: B.c
Out[9]: <bound method A.c of <class '__main__.B'>>
In [10]: A.s
Out[10]: <function __main__.A.s(x)>
In [11]: B.s
Out[11]: <function __main__.A.s(x)>
This suggests that A.c and B.c change depending on the class context, and thus I would expect a separate cache. However, A.s and B.s are exactly the same and thus I would expect a shared cache. To give things a bit more force:
In [13]: A.c == B.c
Out[13]: False
In [14]: A.s == B.s
Out[14]: True
Anyway, I'm willing to make a pull request to fix this, but I'm wondering what your thoughts are on this issue? Should staticmethods on subclassed classes share or not share the superclass' cache?