gh-141645: Add a TUI mode to the new tachyon profiler#141646
gh-141645: Add a TUI mode to the new tachyon profiler#141646pablogsal merged 28 commits intopython:mainfrom
Conversation
8cb430e to
8731b25
Compare
|
CC @lkollar |
|
@hugovk @AA-Turner @savannahostrowski any chance you could give this a go? Most of the diff is tests. You can focus on the main TUI code if you prefer. Even if you just test it locally it will help a lot! |
3922fb1 to
ab80fbf
Compare
lkollar
left a comment
There was a problem hiding this comment.
This is super cool! I've not reviewed the implementation thoroughly but I played around with the TUI and everything is working really well. Left a bunch of suggestions.
Co-authored-by: László Kiss Kollár <kiss.kollar.laszlo@gmail.com>
Co-authored-by: László Kiss Kollár <kiss.kollar.laszlo@gmail.com>
Co-authored-by: László Kiss Kollár <kiss.kollar.laszlo@gmail.com>
|
@lkollar i fix all your comments and clean the code a bit and added even more tests. I also added some color diffing to make it easier to spot changes. Can you give it another go? |
savannahostrowski
left a comment
There was a problem hiding this comment.
This is so slick 🔥 ! I played with this a bunch and the UX is very solid. I have one comment about an edge case I ran into and one suggestion (but it looks like you're already thinking about export so that's great)!
|
When you're done making the requested changes, leave the comment: |
|
@savannahostrowski I have addressed your comments and I pushed a new one ensuring the TUI (most of it) still works at the end in a way that makes sense. I left some comments regarding the exporting situation but we can surely explore that on a different PR :) Thanks a lot for the review ❤️ |
savannahostrowski
left a comment
There was a problem hiding this comment.
Tested out the fix for toggling help when profiling ends and it looks good!
Nice work on this one! Very, very cool! 🚀
This comment was marked as off-topic.
This comment was marked as off-topic.
|
test_profiling tests fail on buildbots installing Python. Example: https://buildbot.python.org/#/builders/207/builds/6809 cc @encukou |
I think we are missing something in the makefile will take a look |
Fixed in #141820 |
Uh oh!
There was an error while loading. Please reload this page.