Closed jonathangjertsen closed 5 years ago
This is a cute idea. My priority is to keep the interface as simple as possible, so if you'd like me to merge it, I'd like you to modify it so it's a decorator on the class rather than a metaclass. Submit a PR like that, with tests and documentation, and I'll likely merge it.
I considered doing this for snoop, I just wanted to hear other people's thoughts on it first: https://github.com/alexmojaki/snoop/issues/2
The main problem with the kind of implementation above (metaclass or not) is that decorated methods in the class won't be traced correctly. If you have a traditional decorator:
def my_decorator(f):
def wrapper(*args, **kwargs):
...
return wrapper
And a method:
@my_decorator
def my_method
then the metaclass above will trace wrapper
instead of my_method
. In fact it'll add it to the target codes which will cause it to be logged if you call any function (even untraced) that has the same decorator when the tracer is active.
OK @cool-RR, I'll give it a shot.
@alexmojaki That's an interesting point, not sure how pysnooper interacts with other decorators in the first place.
not sure how pysnooper interacts with other decorators in the first place.
PySnooper does nothing to specially handle or be aware of other decorators. If you do this:
@snoop()
@my_decorator
def my_method
then you will snoop wrapper
. Sometimes that might even be a good thing if you want to check the behaviour of my_decorator
in a particular case. If it's not, hopefully most users can diagnose the problem and swap the order:
@my_decorator
@snoop()
def my_method
In fact as they consider adding @snoop()
to a decorated function they are likely to notice the potential problem and get it right first time if they think about it and understand how decorators work. There could also potentially be a note in the docs like:
If your function has other decorators, you probably want to apply
@snoop()
first, i.e. at the bottom of the decorator list, unless you want to debug those decorators.
We could perhaps add some code to address this, e.g. to show a warning if @snoop
doesn't appear last.
The problem is a bit different for some other decorators like builtins, e.g:
@snoop()
@property
def my_method
Then currently you get an error like
AttributeError: 'property' object has no attribute '__code__'
which again users can probably diagnose fairly easily. The same thing happens with @staticmethod
, @lru_cache()
, etc.
If you decorate an entire class though, the situation becomes quite different. It's much more likely that at least some of the methods of a class have decorators compared to single function having one. The user is likely to forget that those decorators are there (especially in long classes with many methods) and realise the implications, even after seeing strange debugging output. If they do see the problem, there's no easy way to fix it like before.
Your implementation would need to specially handle each unusual decorator like property
to trace the underlying functions or not trace them at all. And if it's not traced, then the user is very likely to be confused when a method call isn't logged anywhere. They might conclude that the method isn't actually being called at all, which could lead them down a frustrating rabbit hole. This is what a debugger should avoid doing.
Another problem:
import pysnooper
@pysnooper.snoop()
class A:
@pysnooper.snoop()
def foo(self):
pass
A().foo()
Output:
Source path:... /Users/alexhall/Desktop/python/snoop/venv/lib/python3.7/site-packages/pysnooper/tracer.py
Starting var:.. args = (<__main__.A object at 0x104f39b38>,)
Starting var:.. kwargs = {}
Starting var:.. function = <function A.foo at 0x105557950>
Starting var:.. self = <pysnooper.tracer.Tracer object at 0x104f39ac8>
21:27:56.248748 call 250 def simple_wrapper(*args, **kwargs):
21:27:56.255392 line 251 with self:
21:27:56.260776 line 252 return function(*args, **kwargs)
Source path:... /Users/alexhall/Library/Preferences/PyCharm2019.2/scratches/scratch_441.py
Starting var:.. self = <__main__.A object at 0x104f39b38>
21:27:56.262119 call 7 def foo(self):
21:27:56.262523 line 8 pass
21:27:56.262618 return 8 pass
Return value:.. None
Call ended by exception
I didn't find a way to easily snoop on all the methods of a class without adding the decorator to every single method. Below is an outline of a way to do it with a metaclass. There was an issue overwriting the magic/dunder methods (
AttributeError: type object 'SnoopedMetaClass' has no attribute '__code__'
), but for everything else it works fine.