Skip to content
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

Jump to bottom

Argparse choices should be a sequence #92446

Open
rhettinger opened this issue May 8, 2022 · 3 comments · May be fixed by #94627
Open

Argparse choices should be a sequence #92446

rhettinger opened this issue May 8, 2022 · 3 comments · May be fixed by #94627
Labels
3.10 3.11 3.12 docs Documentation in the Doc dir easy

Comments

@rhettinger
Copy link
Contributor

rhettinger commented May 8, 2022

Documentation

Instead of saying "any container" is supported, refer only to "sequences".

Technically, a Container is only required to support __contains__ which is insufficent for argparse. Also, a sets do get accepted are a bad choice because the order shown in help and usage is non-deterministic. So, Sequence is the only reasonable choice because we need sizing and ordered iteration.
@rhettinger rhettinger added docs Documentation in the Doc dir 3.11 easy 3.10 3.9 3.12 labels May 8, 2022
180909 added a commit to 180909/cpython that referenced this issue May 8, 2022
@180909
pythongh-92446: Argparse choices should be a sequence
b36f54c
@180909 180909 mentioned this issue May 8, 2022
Merged
rhettinger pushed a commit that referenced this issue May 13, 2022
@180909
gh-92446: Argparse choices should be a sequence (#92450)
b39f841
@slateny slateny closed this as completed May 15, 2022
@slateny
Copy link
Contributor

slateny commented May 15, 2022

Actually, the linked pr changes refer to lzma instead of argparse, was this intended?

@slateny slateny reopened this May 15, 2022
@AA-Turner
Copy link
Member

AA-Turner commented May 16, 2022 •
edited

On a quick review I'd also say that the change to lzma.rst should be reverted, as the option is in the context of .xz or .lzma container formats.

A

@ulisesojeda ulisesojeda mentioned this issue Jul 5, 2022
Closed
This was referenced Jul 6, 2022
Closed
Open
@DanielNoord
Copy link
Contributor

DanielNoord commented Jul 7, 2022

Both #94561 and #94627 fix this.
The latter seems better though as it is a little more thorough.

I don't think any more PRs are needed, let's focus on merging either of these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.10 3.11 3.12 docs Documentation in the Doc dir easy
Projects
Argparse issues
Status: Doc issues
Milestone
No milestone
5 participants
@rhettinger @AA-Turner @DanielNoord @erlend-aasland @slateny