-
-
Notifications
You must be signed in to change notification settings - Fork 506
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
Debugger widget #2548
Debugger widget #2548
Conversation
This looks great! However it looks like you started with an old(er) checkout and simply undid a bunch of recent changes when you rebased. |
Yes I have noticed the mistake but didn't have time to fix. I added a
comment about it, I'll fix the merge conflict next time.
|
Exceptions are raised after being logged. Starting show from cli will now work
I can't get to have an exception accumulated in a notebook cell, even with master branch & 0.11.3. Not sure how |
Would be great to take this opportunity to add a bunch more log statements throughout the codebase as. The places you identified are definitely good but there's probably a bunch more that would help. |
I identified the functions called whenever a callback is fired. I would use |
Codecov Report
@@ Coverage Diff @@
## master #2548 +/- ##
========================================
Coverage 83.36% 83.37%
========================================
Files 196 198 +2
Lines 26486 26776 +290
========================================
+ Hits 22081 22325 +244
- Misses 4405 4451 +46
Continue to review full report at Codecov.
|
No that makes sense. Just wondering if we can have a few log levels. I'm also thinking of adding an |
A bit like in jupyterhub, that could be helpful indeed. But that could be left at the discretion of the developing user, all the tools are already there to do such a thing. |
New version broke catching callbacks in jupyter
Sorry I left this so late to review, I've cleaned this up a bit and changed it to subscribe to the main panel logger. I'm still a little bit concerned about the brittleness of the width/height/sizing_mode handling which was pretty broken when I first looked at this. I've had to make some changes to terminal sizing but I don't think this handles every combination of min/max_width/height options and different sizing modes correctly. I do want to get a 0.13.0 RC release out today. Will try to clean this up more and get it in. |
Philipp, thanks for your review & corrections. Some features that my experience with my user case made me add:
To do:
There are still some problems with terminal itself where opening and closing the card. The text sometimes appear and sometimes disappear. Interestingly, exporting the log by clicking on the floppy disk button shows the full output even if the terminal is completely black. After testing a bit more, I will open a separate issue as it's not related to this widget but the interaction of Besides the js console keeps complaining about the missing buttons when the card is closed. Not sure if a callback can be "deactivated" whenever the Card is closed so that we don't have the warning in the console. |
@philippjfr I have tested your changes. Very nice. I could not make sizing errors and the debugger now works in a jupyter context. I have added a couple options (logging formatting + decide which logger to prompt) and fixed documentation. I allowed myself to ping you for a review. There is a left over Besides the latter, to me it is ready to merge. Thanks :) |
Added debugger widget. Fixes #2062
NB: only interesting change in reactive.py is within
_process_events
Sample code to try out:
To-do: