-
-
Notifications
You must be signed in to change notification settings - Fork 30k
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
bpo-42572: Improve argparse docs for the type parameter. #23849
Conversation
Also see bpo-37572 and bpo-35005.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a great improvement, Raymond. Thanks!
Doc/library/argparse.rst
Outdated
Even :class:`~argparse.FileType` has its limitations for use with the ``type`` | ||
keyword. If two arguments use *FileType* and the second one fails, an error is | ||
reported but the first file is not automatically closed. In this case, it would | ||
be better to wait until after the parser has run and then use the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this true of any type errors, not just with FileType? If I have a FileType and then a float, and the type conversion for the float fails, I'd still leave the file open. I'm worried that the readers might think this is solely related to the second argument type also being FileType.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. That makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Thanks again.
I have one nit that I forgot to include on the first go-round. That's the only issue I see.
Doc/library/argparse.rst
Outdated
>>> parser.add_argument('bar', type=open) | ||
>>> parser.parse_args('2 temp.txt'.split()) | ||
Namespace(bar=<_io.TextIOWrapper name='temp.txt' encoding='UTF-8'>, foo=2) | ||
If the type_ keyword is used with default_ keyword, the type converter is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be "... is used with the default keyword ..." (adding "the").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
Thanks @rhettinger for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9. |
GH-23869 is a backport of this pull request to the 3.9 branch. |
bpo-42572: Improve argparse docs for the type parameter. (pythonGH-23849)
See https://bugs.python.org/issue42572 and bpo-37572 and bpo-35005.
https://bugs.python.org/issue42572