Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-134861: Add CSV and 🍌SV output formats toasyncio ps#134862

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
dpdani wants to merge7 commits intopython:main
base:main
Choose a base branch
Loading
fromdpdani:feature/pending-tasks-csv-bsv

Conversation

dpdani
Copy link
Contributor

@dpdanidpdani commentedMay 28, 2025•
edited by bedevere-appbot
Loading

@ZeroIntensity
Copy link
Member

(Skipping news because this is an easter egg)

dpdani reacted with heart emojijohnzhou721 reacted with rocket emoji

@johnzhou721
Copy link
Contributor

@ZeroIntensity Hmm... But isn't the CSV format legit, so in my (very bad) opinion it still need news?

@ZeroIntensity
Copy link
Member

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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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.

dpdani reacted with thumbs up emoji
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@danielhollas I would agree to change the argument to🍌 sv. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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 🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks@danielhollas, I agree.@dpdani please hide this from the argparse help (in whatever form the argument ends up being;'🍌' /'🍌sv' /'bsv')

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yep... but bsv is too generic. Wewant to be hacky, the hackier the harder for users to find and the greater joy it brings to the Python community.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I don't feel that hacking our way through argparse for the purpose of hiding this option would be wise, so I would just keep it as-is.

If there are no strong objections I would go ahead and resolve this discussion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

You could use a metavar:metavar="{table,csv}" which hides it from help.

$ python tmp.py --format tsv  usage: tmp.py [-h] [--format {table,csv}]tmp.py: error: argument --format: invalid choice:'tsv' (choose from table, csv, bsv)

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Cool! I didn't know about that, I'll make the change tomorrow 👍

gpshead and johnzhou721 reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Done ✅

I've decided to keep the option asbsv because emojis might be funky for some terminals, and in general are harder to type out.

This is the new behavior:

$ ./python.exe -m asyncio ps 9473 --help        usage: python3 -m asyncio ps [-h] [--format {table,csv}] pidpositional arguments:  pid                   Process ID to inspectoptions:  -h, --help            show this help message and exit  --format {table,csv}$ ./python.exe -m asyncio ps 9473 --format bsv tid🍌task id🍌task name🍌coroutine chain🍌awaiter name🍌awaiter id38847🍌0x1013265d0🍌Task-2🍌🍌🍌0x0

As pointed out above giving a wrong format gives a hint to the big secret 🍌

$ ./python.exe -m asyncio ps 9473 --format ciaousage: python3 -m asyncio ps [-h] [--format {table,csv}] pidpython3 -m asyncio ps: error: argument --format: invalid choice: 'ciao' (choose from table, csv, bsv)

@AA-Turner
Copy link
Member

@ZeroIntensity Hmm... But isn't the CSV format legit, so in my (very bad) opinion it still need news?

Yes, please add news for CSV format

dpdani reacted with thumbs up emoji

Copy link
Contributor

@johnzhou721johnzhou721 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yep, agreed with the NEWS part. But might it make the Easter Egg too obvious? Since people can just look at the PR through news. I think we might be able to split up the PRs into CSV and BSV and skip news on the latter, but it might be too much work.

class TaskTableOutputFormat(Enum):
table = "table"
csv = "csv"
bsv = "bsv"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@AA-Turner
Copy link
Member

AA-Turner commentedMay 29, 2025•
edited
Loading

Yep, agreed with the NEWS part. But might it make the Easter Egg too obvious? Since people can just look at the PR through news. I think we might be able to split up the PRs into CSV and BSV and skip news on the latter, but it might be too much work.

We're not trying to hide or obsfucate; just not to advertise.

ZeroIntensity and dpdani reacted with thumbs up emoji

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@python-cla-bot
Copy link

python-cla-botbot commentedMay 29, 2025•
edited
Loading

All commit authors signed the Contributor License Agreement.

CLA signed

dpdani reacted with thumbs up emoji

@johnzhou721
Copy link
Contributor

johnzhou721 commentedMay 29, 2025 via email

Sure, resolve.
dpdani reacted with thumbs up emoji

johnzhou721

This comment was marked as outdated.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@AA-TurnerAA-TurnerAA-Turner left review comments

@danielhollasdanielhollasdanielhollas left review comments

@nineteendonineteendonineteendo left review comments

@johnzhou721johnzhou721johnzhou721 requested changes

@1st11st1Awaiting requested review from 1st11st1 is a code owner

@asvetlovasvetlovAwaiting requested review from asvetlovasvetlov is a code owner

@kumaraditya303kumaraditya303Awaiting requested review from kumaraditya303kumaraditya303 is a code owner

@willingcwillingcAwaiting requested review from willingcwillingc is a code owner

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@dpdani@ZeroIntensity@johnzhou721@AA-Turner@danielhollas@nineteendo
[8]ページ先頭
©2009-2025 Movatter.jp