Repository: cool-RR/PySnooper

Issue:
Feature request: Support generators
See the conversation on #31 .

Discussion:
cool-RR: Good work.

1. functools.wraps isn't as strong as the decorator module, so please use the latter.
2. Why extras_require?
3. Generators and coroutines should have the context manager acquired only when active.
4. Add tests.
5. Optional: move the python 3 syntax into a string that's exec'd conditionally if the python version is high enough. I can do this for you if you prefer, after you do the other items.
pohmelie: 1. What is the reason for `decorator` module here? Why `funtools.wraps` is «weak» in this case?
2. It is a common practice in python packaging to have `extras` for tests. So you can install just package or install package + tests dependencies with `pip install pysnooper[tests]`. Good for CI and devs.
3. And... they actually do. What do you mean?
4. Tests are too foggy for me, not sure that I will have time and passion to learn how they work.
5. I prefer you to do this.
cool-RR: 1. Because it doesn't update the signature.
2. Makes sense, but I think that `tests_require` is better for this. That's what I originally used, but @bittner removed it, and he's more knowledgeable and testing and CI than me. I suggest you remove this change from this PR and if you're interested, start a separate PR/issue/discussion.
3. No, in your code, the context manager gets acquired when the generator is first called, and it'll only get released when the generator gave all the items. (Assuming it's even finite.) I want the context manager to be acquired every time when the generator is resumed, and released whenever the generator gets paused. Same for coroutines.
4. Okay, just try it manually on a code snippet and show it to me, and I'll do the tests for you. 
5. Sure, after there's more progress on the items above.
pohmelie: 1. What signatures are you talking about? Docstring? Arguments? Name?
2. Ok. Sounds reasonable. But, for a `tests_require`, it is poor choice, since this will work only as `python setup.py test`, and you should write custom code in your `setup.py` for this command which, surprisingly, should call `pytest` as subprocess. :laughing: 
3. At first, this is not possible for coroutines. At second, what is the reason for this?
4. https://gist.github.com/pohmelie/f14aea0ddec4493f0ce2da7f3ddc28ac
cool-RR: 1. Arguments, defaults, information about which arguments are keyword-only, positional-only or regular.
2. Good :)
3. The reason is that the generator/coroutine may be paused for an unlimited amount of time, and a trace function is quite an invasive thing, both because of performance and because it might intefere with other trace functions. Therefore I want to have the trace function enabled for the bare minimum scope possible. Regarding it not being possible for coroutines: @Nurdok Can you weigh in on this? 
4. Great, thanks! 
pohmelie: 1. That is exactly what `wraps` do. No difference with `decorator` module in this case. Except that `wraps` maintained by python devs, this is a big advantage. https://repl.it/@multisosnooley/PoliticalUnrulyPlots
alexmojaki: 1. I don't have time to investigate or say much but I think wraps only adds the signature in recent Python versions. Also, maybe consider `wrapt`.
cool-RR: @pohmelie This doesn't work on Python 2.7. In any case, even if we were to change to `wraps` in the future, that'll be on a separate PR.
pohmelie: @alexmojaki, @cool-RR 
Checked that. It looks like you are right. Hopefully, I have no experience with 2.7 at all.
AtHeartEngineer: > @pohmelie This doesn't work on Python 2.7. In any case, even if we were to change to `wraps` in the future, that'll be on a separate PR.

It's cool to support 2.7 but it's about to be end of life. Maybe duplicate master as 'a python 2.7' version and just leave it as is, then use 'wraps' in master moving forward? For maintaining old code I totally get it, but with 2to3, if you are going to do maintenance on old code, you should convert it to python 3.5 or 3.6 at least. Just my opinion, definitely appreciate the library no matter what!
cool-RR: @AtHeartEngineer I'll be as happy as anyone to take Python 2.7 out back and shoot it in the back of the head, but unfortunately, PySnooper was meant for the shittiest of shitty corporate environments. That's why it should support Python 2.7 for the foreseeable future, and not have dependencies.

Also, if @pohmelie lost motivation on this PR, feel free to take over.
pohmelie: @cool-RR what do you mean by "lost motivation"? You said that you will make exec part and tests. My contribution part is done and all issues resolved.
cool-RR: @pohmelie Ah sorry, looks like we had a miscommunication. I'll review your code soon.
alexmojaki: Point 3 was a bit of a problem, I was also hoping someone could confirm that it's not possible to restrict the scope of the tracer in coroutines.
cool-RR: @pohmelie I'm consulting someone about the coroutine case, but here's a bug you have for the generators: You need to deal with the case of a generator that also receives values from its caller, and possibly gets exceptions thrown by its caller. You need to propagate those. 

See https://docs.python.org/3/reference/expressions.html#generator.send and https://docs.python.org/3/reference/expressions.html#generator.throw
pohmelie: @cool-RR nice catch about gens, forget this part actually.
cool-RR: 1. You need to rebase this PR correctly. Possibly sqaush your commits for easier handling, because the diff right now is wrong.
2. A friend of mine suggested that the coroutine thing I asked for is possible using contextvars, can you check it out? https://groups.google.com/d/msg/pyweb-il/HdgEdvFUF9M/jI8Gt3bQAgAJ
3. Another friend noticed that this implementations of generators is wrong, because the `yield` is inside the `with` statement, causing it to stay acquired when the generator gets paused. Here is a fix (which doesn't take into account your previous fix so you can't just copy-paste the code): https://groups.google.com/forum/#!topic/pyweb-il/HdgEdvFUF9M
pohmelie: 1. The [diff](https://github.com/cool-RR/PySnooper/pull/65/files) is just perfect! Couple of lines in one file, actually in one method — `__call__`. What do you mean?
2. I presume your friend is wrong. You can't detect that you go out from coroutine, so you can't release context. It is just same plain `yield from`, which means «passthrough all data from inner to outer, I don't care what it is». Anyway if he can provide working solution to detect and release context in a moment when we pass value to outer, then I would be just happy.
3. Nice catch!
cool-RR: 1. I see that the `diff` is looking good now, possibly I was mistaken before. 
2. Okay, I'll investigate this. 
3. Good job.
4. You need to add tests.
5. Later when we'll have more confidence, we'll add documentation.
pohmelie: > Okay, just try it manually on a code snippet and show it to me, and I'll do the tests for you.

Actually you agreed to write tests by your own before. :smile: 
cool-RR: Yes, you're right, I'll write the tests. Can you confirm that you tried your code with generators and coroutines and it seemed to work?
pohmelie: @cool-RR 

Yes, there is a quick test snippet above which I use to test generators and coroutines.
cool-RR: Awesome, I'll get to work on this. Thanks!
cool-RR: @pohmelie Can you please add me as a collaborator on https://github.com/pohmelie/PySnooper/ ?
pohmelie: I don't need to add you as collaborator, actually you already have permissions to push to my branch.
cool-RR: @pohmelie I decided to separate generators from coroutines, because the latter are more difficult and much less important for most users. 

Patch:
diff --git a/README.md b/README.md
index 331488c..9b43596 100644
--- a/README.md
+++ b/README.md
@@ -149,6 +149,9 @@ On multi-threaded apps identify which thread are snooped in output::
 @pysnooper.snoop(thread_info=True)
 ```
 
+PySnooper supports decorating generators.
+
+
 # Installation #
 
 You can install **PySnooper** by:
diff --git a/pysnooper/pycompat.py b/pysnooper/pycompat.py
index 4f2a9a6..b63df9d 100644
--- a/pysnooper/pycompat.py
+++ b/pysnooper/pycompat.py
@@ -4,6 +4,7 @@
 
 import abc
 import os
+import inspect
 
 if hasattr(abc, 'ABC'):
     ABC = abc.ABC
@@ -35,3 +36,9 @@ else:
                 (hasattr(subclass, 'open') and
                  'path' in subclass.__name__.lower())
             )
+
+
+try:
+    iscoroutinefunction = inspect.iscoroutinefunction
+except AttributeError:
+    iscoroutinefunction = lambda whatever: False # Lolz
diff --git a/pysnooper/tracer.py b/pysnooper/tracer.py
index 844809d..b05b0f9 100644
--- a/pysnooper/tracer.py
+++ b/pysnooper/tracer.py
@@ -209,11 +209,32 @@ class Tracer:
         self.target_codes.add(function.__code__)
 
         @functools.wraps(function)
-        def inner(*args, **kwargs):
+        def simple_wrapper(*args, **kwargs):
             with self:
                 return function(*args, **kwargs)
 
-        return inner
+        @functools.wraps(function)
+        def generator_wrapper(*args, **kwargs):
+            gen = function(*args, **kwargs)
+            method, incoming = gen.send, None
+            while True:
+                with self:
+                    try:
+                        outgoing = method(incoming)
+                    except StopIteration:
+                        return
+                try:
+                    method, incoming = gen.send, (yield outgoing)
+                except Exception as e:
+                    method, incoming = gen.throw, e
+
+        if pycompat.iscoroutinefunction(function):
+            # return decorate(function, coroutine_wrapper)
+            raise NotImplementedError
+        elif inspect.isgeneratorfunction(function):
+            return generator_wrapper
+        else:
+            return simple_wrapper
 
     def write(self, s):
         if self.overwrite and not self._did_overwrite:


Test patch:
diff --git a/tests/test_pysnooper.py b/tests/test_pysnooper.py
index d3882e4..4691cfb 100644
--- a/tests/test_pysnooper.py
+++ b/tests/test_pysnooper.py
@@ -5,6 +5,7 @@ import io
 import textwrap
 import threading
 import types
+import sys
 
 from pysnooper.utils import truncate
 from python_toolbox import sys_tools, temp_file_tools
@@ -1047,3 +1048,86 @@ def test_indentation():
 def test_exception():
     from .samples import exception
     assert_sample_output(exception)
+
+
+def test_generator():
+    string_io = io.StringIO()
+    original_tracer = sys.gettrace()
+    original_tracer_active = lambda: (sys.gettrace() is original_tracer)
+
+
+    @pysnooper.snoop(string_io)
+    def f(x1):
+        assert not original_tracer_active()
+        x2 = (yield x1)
+        assert not original_tracer_active()
+        x3 = 'foo'
+        assert not original_tracer_active()
+        x4 = (yield 2)
+        assert not original_tracer_active()
+        return
+
+
+    assert original_tracer_active()
+    generator = f(0)
+    assert original_tracer_active()
+    first_item = next(generator)
+    assert original_tracer_active()
+    assert first_item == 0
+    second_item = generator.send('blabla')
+    assert original_tracer_active()
+    assert second_item == 2
+    with pytest.raises(StopIteration) as exc_info:
+        generator.send('looloo')
+    assert original_tracer_active()
+
+    output = string_io.getvalue()
+    assert_output(
+        output,
+        (
+            VariableEntry('x1', '0'),
+            VariableEntry(),
+            CallEntry(),
+            LineEntry(),
+            VariableEntry(),
+            VariableEntry(),
+            LineEntry(),
+            ReturnEntry(),
+            ReturnValueEntry('0'),
+
+            # Pause and resume:
+
+            VariableEntry('x1', '0'),
+            VariableEntry(),
+            VariableEntry(),
+            VariableEntry(),
+            CallEntry(),
+            VariableEntry('x2', "'blabla'"),
+            LineEntry(),
+            LineEntry(),
+            VariableEntry('x3', "'foo'"),
+            LineEntry(),
+            LineEntry(),
+            ReturnEntry(),
+            ReturnValueEntry('2'),
+
+            # Pause and resume:
+
+            VariableEntry('x1', '0'),
+            VariableEntry(),
+            VariableEntry(),
+            VariableEntry(),
+            VariableEntry(),
+            VariableEntry(),
+            CallEntry(),
+            VariableEntry('x4', "'looloo'"),
+            LineEntry(),
+            LineEntry(),
+            ReturnEntry(),
+            ReturnValueEntry(None),
+
+        )
+    )
+
+
+