cool-RR / PySnooper

Never use print for debugging again
MIT License
16.39k stars 954 forks source link

Allow using PySnooper as a command line program #145

Closed tebeka closed 5 years ago

tebeka commented 5 years ago

A POC for running pysnooper as an external command (e.g. pysnooper /path/to/script.py)

The script will create sitecustomize.py script and will add it first in PYTHONPATH.

cool-RR commented 5 years ago

Interesting. Did you find yourself wanting this functionality in a real-world scenario?

tebeka commented 5 years ago

I haven't used PySnooper is real world scenario. However first time I played with PySnooper I was looking for something like this to inspect my code without modifying it.

tebeka commented 5 years ago

If the concept is appvroved, I'll work on what's left:

cool-RR commented 5 years ago

This sounds great, consider it approved.

@alexmojaki if you have an opinion about this, I'll be happy to hear it, and of course feel free to merge it to your fork if you're interested.

cool-RR commented 5 years ago

@tebeka If you get stuck on anything or want me to help implement anything, let me know!

alexmojaki commented 5 years ago

I haven't tried it myself but in its current form it looks like it would only trace the created sitecustomize.py, so I'm a bit confused. Does this work?

I think it's best that this branch gets used in a real world scenario first. The benefit of not modifying source code is pretty minimal, and this seems like it would be rarely useful. It's not often that you want to start tracing from the top level, and it may take rather high depth to get where you want, which will create lots of noise. Perhaps it would be better if you could specify the name of one or more functions to trace, to mimic the decorator.

alexmojaki commented 5 years ago

Maybe a good alternative method to not alter the source would be environment variable activation similar to better-exceptions. There are plenty of situations when a custom run command is not an option, especially if you're already using a command from a different library (e.g. pytest). @tebeka what would you think of that?

cool-RR commented 5 years ago

@alexmojaki I hope I can take credit for making you more cautious about adding potentially esoteric functionality :)

@tebeka If you get it working right and well-tested, I'll merge it.

tebeka commented 5 years ago

@alexmojaki

I haven't tried it myself but in its current form it looks like it would only trace the created sitecustomize.py, so I'm a bit confused. Does this work?

It works since I start the snooper at sitecustomize.py (which is loaded by Python) and do not close it.

The benefit of not modifying source code is pretty minimal,

I disagree :) Most profiling/debugging tools work without requiring the user to modify their code. I would't like to set breakpoint by manually adding breakpoint() at my code.

It's not often that you want to start tracing from the top level, and it may take rather high depth to get where you want

I agree that in the future we might add a way to start tracing only from the user script or many support some command line options to specify which functions to monitor (some kind of filter probably). But let's start with a simple thing and move on from there.

Maybe a good alternative method to not alter the source would be environment variable activation

This will still require the user to change their source code.

There are plenty of situations when a custom run command is not an option, especially if you're already using a command from a different library (e.g. pytest).

Your right in the general case, but I try to solve a specific case and not a general one. In the case of pytest this can be done via a plugin.

tebeka commented 5 years ago

Hmm, for some reason I don't see the traced program output anymore (like @alexmojaki said). Probably due to merge with master. Will investigate

alexmojaki commented 5 years ago

This will still require the user to change their source code.

No it wouldn't. It sounds like you didn't look at what better-exceptions does. It provides a .pth file that automatically activates when you specify an environment variable. Another example which is more like PySnooper is hunter.

tebeka commented 5 years ago

@cool-RR Can you take a look now?

alexmojaki commented 5 years ago

This implementation seems very complicated - sitecustomize, subprocess, environment variables...

Try something like this:

code_obj = compile(source, filename, 'exec')
tracer.target_codes.add(code_obj)
exec(code)

That's roughly how the IPython integration of snoop works: https://github.com/alexmojaki/snoop/blob/master/snoop/ipython.py

cool-RR commented 5 years ago

@tebeka I reviewed the code and found a couple of problems:

  1. The _should_trace refactoring is elegant, but I think it might hurt performance. It's important that for lines that shouldn't be traced, the trace function will return as quickly as possible. Every function call matters because the trace function could be called millions of times.
  2. I'm a little wary about the fact that your test finds the Python executable and runs it in a separate process. I think that maybe things can go wrong that way. Is there no way to test it using exec or something.

Also, Alex's input above. Also, the tests fail.

If you find this too overwhelming, I'd understand if you'd abandon the PR. I think its value is marginal, so I'd only accept it if it's really well done.

tebeka commented 5 years ago

@alexmojaki

This will still require the user to change their source code.

No it wouldn't. It sounds like you didn't look at what better-exceptions does. It provides a .pth file that automatically activates when you specify an environment variable. Another example which is more like PySnooper is hunter.

You're right, my bad - Sorry.

However I dislike things the globally change everything in the Python interpreter. Also the code in the .pth looks pretty ugly to me :)

code_obj = compile(source, filename, 'exec')

Most cases for command line script you'd like to pass some parameters. This becomes difficult in thsis approach.

tebeka commented 5 years ago

@cool-RR

The _should_trace refactoring is elegant, but I think it might hurt performance. It's important that for lines that shouldn't be traced, the trace function will return as quickly as possible. Every function call matters because the trace function could be called millions of times.

I understand the concern. However this statement needs to be backed up by profiling :)

I'm a little wary about the fact that your test finds the Python executable and runs it in a separate process. I think that maybe things can go wrong that way. Is there no way to test it using exec or something.

I'm trying to test the actual script. If things go wrong - it's probably a bug.

Also, Alex's input above. Also, the tests fail. Answered Alex. Will look at test.

If you find this too overwhelming, I'd understand if you'd abandon the PR. I think its value is marginal, so I'd only accept it if it's really well done. Totally understand. Seems from your & @alexmojaki comment that this PR is not well aligned with the project. I'm closing it for now.

alexmojaki commented 5 years ago

Hey @tebeka, thanks for your time. I suggest using PySnooper (or snoop) in a real scenario a few times to understand what using it is like. If you find a situation where you'd really like this feature let us know and we might be convinced to add it. I might even implement it myself.

There's also the possibility of tracing many functions without putting @snoop on each one, so you modify some source but not too much. I've suggested some possibilities here and would be interested in your opinion.

cool-RR commented 5 years ago

Recently I found myself wanting this feature a few times, so I think it's a pretty good idea now.

If anyone were interested in implementing it according to our notes above, that'd be cool.