Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(5241)

#11176: give more meaningful argument names in argparse documentation

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 months, 3 weeks ago by steven.bethard
Modified:
10 months, 2 weeks ago
Reviewers:
merwok, ezio.melotti
CC:
bethard, ezio.melotti, eric.araujo, sandro.tosi, cjerdonek, docs_python.org, marcs, anikom15, tshepang, Adam Woodbeck, dlam, groodt_gmail.com
Visibility:
Public.

Patch Set 1 #

Total comments: 3

Patch Set 2 #

Total comments: 9

Patch Set 3 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/argparse.rst View 1 2 4 chunks +62 lines, -45 lines 0 comments Download

Messages

Total messages: 3
eric.araujo
A few minor comments. http://bugs.python.org/review/11176/diff/5329/Doc/library/argparse.rst File Doc/library/argparse.rst (right): http://bugs.python.org/review/11176/diff/5329/Doc/library/argparse.rst#newcode52 Doc/library/argparse.rst:52: print("Report will be written to: ...
10 months, 2 weeks ago #1
eric.araujo
A few minor comments.
10 months, 2 weeks ago #2
ezio.melotti
10 months, 2 weeks ago #3
http://bugs.python.org/review/11176/diff/5362/Doc/library/argparse.rst
File Doc/library/argparse.rst (right):

http://bugs.python.org/review/11176/diff/5362/Doc/library/argparse.rst#newcode40
Doc/library/argparse.rst:40: parser.add_argument('-f', '--file', type=str,
default="/usr/share/dict/words",
Not sure it's a good idea to provide a default here (especially one that doesn't
work on all the operative systems).

http://bugs.python.org/review/11176/diff/5362/Doc/library/argparse.rst#newcode70
Doc/library/argparse.rst:70: text_file_lines.extend(text_file.readlines())     #
start at "line 1"
I would keep the lines starting from 0, and use the with statement:
with open(args.file) as f:
    lines = list(f)

http://bugs.python.org/review/11176/diff/5362/Doc/library/argparse.rst#newcode74
Doc/library/argparse.rst:74: lines_to_print =
(text_file_lines[args.line_numbers[0]]),
Did you mean to put the comma inside the ()?

http://bugs.python.org/review/11176/diff/5362/Doc/library/argparse.rst#newcode78
Doc/library/argparse.rst:78: lines_to_print =
itemgetter(*args.line_numbers)(text_file_lines)
This is a bit tricky, I would use something like:
[line for n,line in enumerate(lines)
 if n in args.line_numbers]?

http://bugs.python.org/review/11176/diff/5362/Doc/library/argparse.rst#newcode84
Doc/library/argparse.rst:84: print('Line #{0}: 
{1}'.format(args.line_numbers[i], line), end="")
The 0 and 1 are unnecessary.

http://bugs.python.org/review/11176/diff/5362/Doc/library/argparse.rst#newcod...
Doc/library/argparse.rst:207: >>> pizza_parser = argparse.ArgumentParser(
/me approves pizza

http://bugs.python.org/review/11176/diff/5362/Doc/library/argparse.rst#newcod...
Doc/library/argparse.rst:472: ...                     help='the sauce to use')
This should be indented under the first ' of the previous line.

http://bugs.python.org/review/11176/diff/5362/Doc/library/argparse.rst#newcod...
Doc/library/argparse.rst:495: ...     help="how man slices the pizza will be cut
up into")
Why a float? can you cut 7.9 slices?

http://bugs.python.org/review/11176/diff/5362/Doc/library/argparse.rst#newcod...
Doc/library/argparse.rst:1125: >>> pizza_parser.add_argument('slices',
nargs='?', type=int, default=10,
The default here should be 8.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld cbc36f91f3f7