gh-134861: Add CSV and 🍌SV output formats to asyncio ps
#134862
Conversation
dpdani
requested review from
1st1,
asvetlov,
kumaraditya303 and
willingc
as code owners
# Conflicts: # Lib/asyncio/tools.py
|
(Skipping news because this is an easter egg) |
|
@ZeroIntensity Hmm... But isn't the CSV format legit, so in my (very bad) opinion it still need news? |
|
Oh, hm. I guess we could add an entry for only CSV. I'll leave the decision to @dpdani. |
| class TaskTableOutputFormat(Enum): | ||
| table = "table" | ||
| csv = "csv" | ||
| bsv = "bsv" |
There was a problem hiding this comment.
Perhaps for the Easter Egg, make the argument the literal banana emoji, 'bsv' is too generic. Please also add some sort of reference for the future in a comment as to why bananas are relevant.
There was a problem hiding this comment.
There was a problem hiding this comment.
@danielhollas I would agree to change the argument to 🍌 sv. What do you think?
There was a problem hiding this comment.
Copying my comment from #134862 (comment)
This would print the 🍌sv option in the argparse help, right? Is that desired? (I don't care about hiding it, but I think it could be very confusing for users, and also may display weirdly if they don't have a proper font installed).
Don't have a strong opinion 🤷♂️
There was a problem hiding this comment.
Thanks @danielhollas, I agree. @dpdani please hide this from the argparse help (in whatever form the argument ends up being; '🍌' / '🍌sv' / 'bsv')
There was a problem hiding this comment.
Mm that wouldn't be trivial, considering that argparse would error out because the format is not in the list.
Does it really have to be hidden?
There was a problem hiding this comment.
My proposal: Instead of doing --format, do another option --banana that is only in effect when --format csv is used but does not error out when it isn't so it make the easter egg harder to discover. We can use https://stackoverflow.com/questions/37303960/show-hidden-option-using-argparse on that, and have a _banana boolean for it to set, that way we can use elif == <...>.csv (explicit > implicit) on L224 and simplify L240 a lot by simply using a boolean for checking banana and not a switch.
There was a problem hiding this comment.
It feels very hacky to do it though, I'm not sure.
Right now I'm leaning on the current behavior:
$ ./python.exe -m asyncio ps --help
usage: python3 -m asyncio ps [-h] [--format {table,csv,bsv}] pid
positional arguments:
pid Process ID to inspect
options:
-h, --help show this help message and exit
--format {table,csv,bsv}
$ ./python.exe -m asyncio ps 92294 --format bsv
tid🍌task id🍌task name🍌coroutine chain🍌awaiter name🍌awaiter id
4876371🍌0x101c4bdf0🍌Task-4🍌🍌🍌0x0
There was a problem hiding this comment.
Yep... but bsv is too generic. We want to be hacky, the hackier the harder for users to find and the greater joy it brings to the Python community.
Yes, please add news for CSV format |
| class TaskTableOutputFormat(Enum): | ||
| table = "table" | ||
| csv = "csv" | ||
| bsv = "bsv" |
There was a problem hiding this comment.
We're not trying to hide or obsfucate; just not to advertise. |
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
|
All commit authors signed the Contributor License Agreement. |
|
Sure, resolve.
|
| class TaskTableOutputFormat(Enum): | ||
| table = "table" | ||
| csv = "csv" | ||
| bsv = "bsv" |
There was a problem hiding this comment.
My proposal: Instead of doing --format, do another option --banana that is only in effect when --format csv is used but does not error out when it isn't so it make the easter egg harder to discover. We can use https://stackoverflow.com/questions/37303960/show-hidden-option-using-argparse on that, and have a _banana boolean for it to set, that way we can use elif == <...>.csv (explicit > implicit) on L224 and simplify L240 a lot by simply using a boolean for checking banana and not a switch.
asyncio ps#134861