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.

Sign up for GitHub

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

bpo-37726: prefer argparse over getopt #15052

Merged
merged 4 commits into from
Aug 1, 2019
Merged

bpo-37726: prefer argparse over getopt #15052

merged 4 commits into from
Aug 1, 2019

Conversation

mental32
Copy link
Contributor

@mental32 mental32 commented Jul 31, 2019 •
edited by bedevere-bot
Loading

bpo-37726: prefer argparse over getopt in tutorial

The tutorial section on "command line arguments" recommended using getopt for argument parsing and referenced argparse as the fancy alternative.

The topic was raised in the python-ideas by Andrew and Guido created the issue and tagged it with the new "newcomer friendly" tag.

This is my first doc change so I'd greatly appreciate a review and especially criticism, if I did something wrong point it out so I can avoid repeating it.

https://bugs.python.org/issue37726

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Jul 31, 2019
isidentical
isidentical reviewed Jul 31, 2019
View reviewed changes
Doc/tutorial/stdlib.rst Outdated
The :mod:`getopt` module processes *sys.argv* using the conventions of the Unix
:func:`getopt` function. More powerful and flexible command line processing is
provided by the :mod:`argparse` module.
The :mod:`argparse` module provides a mechanism to process command line arguments,
Copy link
Member

Choose a reason for hiding this comment

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

I think sentence should end with .

gvanrossum
gvanrossum approved these changes Jul 31, 2019
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Thanks! I fixed the punctuation and otherwise I'm happy with this. I'll wait 24 hours for other core devs to jump in; if there's no more feedback I'll merge it.

ericvsmith
ericvsmith reviewed Jul 31, 2019
View reviewed changes
Copy link
Member

@ericvsmith ericvsmith left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!
See my comments about keeping with the same style as the rest of the document.

Doc/tutorial/stdlib.rst Outdated

The three lines of code needed for creating the command line argument formula
are simple to read and understand and are easy to extend in terms of your
applications functionality.
Copy link
Member

Choose a reason for hiding this comment

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

I think you should just let the example stand, instead of asserting that it's simple to read and extend.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was attempting to make obvious my point, but if the examples can stand on their own and have the same effect then I'll gladly omit the statement 👍

Doc/tutorial/stdlib.rst Outdated

>>> parser.add_argument('--verbose', '-v', action='count')
>>> greeting = ["Hi", "Hello", "Greetings! its very nice to meet you"][args.verbose % 3]
>>> print(f'{greeting}, {args.name}')
Copy link
Member

Choose a reason for hiding this comment

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

I think you should just include all of these examples in with the first example, and only have a single example, without any additional text. That seems to be how it's done in the rest of the document.

@gvanrossum
Copy link
Member

@mental32 Do you have time to make the changes suggested by Eric? The seem a good idea to me.

@mental32
Copy link
Contributor Author

mental32 commented Aug 1, 2019

@gvanrossum Indeed I do, changes will be pushed in the next twenty minutes :)

matrixise
matrixise approved these changes Aug 1, 2019
View reviewed changes
Copy link
Member

@matrixise matrixise left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution, I think we can merge this PR.

@gvanrossum gvanrossum merged commit 2491134 into python:master Aug 1, 2019
@bedevere-bot
Copy link

@gvanrossum: Please replace # with GH- in the commit message next time. Thanks!

@gvanrossum
Copy link
Member

Thank you @mental32. Your contributions makes a difference!

@miss-islington
Copy link
Contributor

Thanks @mental32 for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @mental32 for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-15069 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 1, 2019
@mental32 @miss-islington
bpo-37726: Prefer argparse over getopt in stdlib tutorial (pythonGH-1…
b873b2f 
…5052)

(cherry picked from commit 2491134)

Co-authored-by: mental <m3nta1@yahoo.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 1, 2019
@mental32 @miss-islington
bpo-37726: Prefer argparse over getopt in stdlib tutorial (pythonGH-1…
30190f9 
…5052)

(cherry picked from commit 2491134)

Co-authored-by: mental <m3nta1@yahoo.com>
@bedevere-bot
Copy link

GH-15070 is a backport of this pull request to the 3.8 branch.

gvanrossum pushed a commit that referenced this pull request Aug 1, 2019
@miss-islington @mental32
bpo-37726: Prefer argparse over getopt in stdlib tutorial (GH-15052) (#…
dcc53eb 
…15070)

(cherry picked from commit 2491134)

Co-authored-by: mental <m3nta1@yahoo.com>
gvanrossum pushed a commit that referenced this pull request Aug 1, 2019
@miss-islington @mental32
bpo-37726: Prefer argparse over getopt in stdlib tutorial (GH-15052) (#…
8990ac0 
…15069)

(cherry picked from commit 2491134)

Co-authored-by: mental <m3nta1@yahoo.com>
@mental32 mental32 deleted the bpo-37726-tutorial-getopt-argparse branch August 1, 2019 21:19
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
@mental32 @lisroach
bpo-37726: Prefer argparse over getopt in stdlib tutorial (python#15052)
b55e6c9
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@mental32 @DinoV
bpo-37726: Prefer argparse over getopt in stdlib tutorial (python#15052)
c4ca02f
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
@mental32 @websurfer5
bpo-37726: Prefer argparse over getopt in stdlib tutorial (python#15052)
a80db64
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericvsmith ericvsmith ericvsmith left review comments

@matrixise matrixise matrixise approved these changes

@gvanrossum gvanrossum gvanrossum approved these changes

@isidentical isidentical isidentical approved these changes

Assignees
No one assigned
Labels
docs Documentation in the Doc dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@mental32 @gvanrossum @bedevere-bot @miss-islington @matrixise @ericvsmith @isidentical @the-knights-who-say-ni