gh-138122: Refactor the CLI of profiling.sampling into subcommands#141813
gh-138122: Refactor the CLI of profiling.sampling into subcommands#141813pablogsal merged 8 commits intopython:mainfrom
Conversation
0790c5b to
1fcfffc
Compare
1fcfffc to
db2ed3b
Compare
|
@savannahostrowski I have reformulated the CLI to break it into sub commands. Could you play with this a bit and tell me what you think? |
|
CC @lkollar |
…nto subcommands
|
@hugovk if you have some time can you play with the CLI and tell be if you see any improvements we could do? |
savannahostrowski
left a comment
There was a problem hiding this comment.
This refactoring makes the UX so much less intimidating off the jump IMO. I really like what it does for help tbh. Makes things a bit more intuitive!
One suggestion around how we can clean up examples in the help but otherwise, LGTM!
lkollar
left a comment
There was a problem hiding this comment.
I played around with it a bit and the subcommands make the CLI much nicer.
One odd thing I noticed is that both run and attach is fine with --pstats and any of the pstats format options with --live. We should probably block these in live mode.
hugovk
left a comment
There was a problem hiding this comment.
We'll need to update the examples at https://docs.python.org/3.15/whatsnew/3.15.html#pep-799-high-frequency-statistical-sampling-profiler but this can also be a followup PR once this is settled.
Looks good, thanks!
| import collections | ||
| import marshal | ||
|
|
||
| from _colorize import ANSIColors |
There was a problem hiding this comment.
Rather than using ANSIColors directly, it would be good to set and use a theme instead, so people can easily customise. Can be a followup PR.
There was a problem hiding this comment.
Rather than using
ANSIColorsdirectly, it would be good to set and use a theme instead, so people can easily customise. Can be a followup PR.
Yeah good point, will do in a separate PR since the live TUI also may need that
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
|
|
will fix this bug |
Uh oh!
There was an error while loading. Please reload this page.