docbuild: switch from optparse to argparse
Closed this issue · 53 comments
Sage's docbuilding uses optparse for argument parsing, but this library has been deprecated for a while. We should switch to argparse.
Some instructions for conversions can be found here: https://docs.python.org/3/library/argparse.html#help
Depends on #32946
Component: documentation
Author: Frédéric Chapoton
Branch/Commit: 0b62c16
Reviewer: John Palmieri
Issue created by migration from https://trac.sagemath.org/ticket/31366
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
Branch: public/ticket/31366
first tentative
New commits:
159ec4b | first step towards using argparse for doc command line arguments |
Branch pushed to git repo; I updated commit sha1. New commits:
56ce7b6 | more work on argparse for doc command line |
Branch pushed to git repo; I updated commit sha1. New commits:
98824cd | Merge branch 'public/ticket/31366' in 9.5.b1 |
Author: Frédéric Chapoton
not quite sure that it works perfectly, but it seems so
needs work
[dochtml] File "/home/sagemath/sage-9.1/local/lib/python3.7/site-packages/sage_docbuild/__init__.py", line 1558, in setup_parser
[dochtml] description=help_description(compact=True))
[dochtml] TypeError: __init__() got an unexpected keyword argument 'add_help_option'
Branch pushed to git repo; I updated commit sha1. New commits:
3a4c572 | more work on argparse for docbuild |
Description changed:
---
+++
@@ -1 +1,3 @@
Sage's docbuilding uses [optparse](https://docs.python.org/3/library/optparse.html) for argument parsing, but this library has been deprecated for a while. We should switch to [argparse](https://docs.python.org/3/library/argparse.html).
+
+Some instructions for conversions can be found here: https://docs.python.org/3/library/argparse.html#helpI'm getting the message AttributeError: module 'argparse' has no attribute 'OptionGroup'. I can't tell what the right replacement is — add_argument_group? add_subparser?
Branch pushed to git repo; I updated commit sha1. New commits:
70b6aa6 | another fix for argparse in docbuild |
here is a tentative. I had no time to check
Branch pushed to git repo; I updated commit sha1. New commits:
914ce7d | more work on argparse for docbuild |
more fixes, but apparently not ok yet
Branch pushed to git repo; I updated commit sha1. New commits:
199f8a0 | trac 31366: more fixes |
I tried implementing this a while ago (before your branch), so here are some contributions to your branch from my attempts. They help, but not sure if they are good enough yet.
I was working on that right now. I had to merge my branch with yours, hopefully in a correct way. Starts to look better.
Yes, much better. We have to figure how to allow commands like ./sage --docbuild -h, which currently says
usage: sage --docbuild [OPTIONS] DOCUMENT (FORMAT | COMMAND)
__main__.py: error: the following arguments are required: document, format
Branch pushed to git repo; I updated commit sha1. New commits:
8c8fc39 | still working on argparse for docbuilding |
more progress done.
and the bot+linter are now green ; still needs tests and double-check, for sure
Looks very close now, docs build and tests pass.
The old version exited gracefully with the help message if you ran ./sage --docbuild, whereas this one prints an error message instead.
Another difference. Old:
% ./sage --docbuild -h
Usage: sage --docbuild [OPTIONS] DOCUMENT (FORMAT | COMMAND)
Build or return information about Sage documentation.
DOCUMENT name of the document to build
FORMAT document output format
COMMAND document-specific command
Note that DOCUMENT may have the form 'file=/path/to/FILE',
which builds the documentation for the specified file.
A DOCUMENT and either a FORMAT or a COMMAND are required,
unless a list of one or more of these is requested.
OPTIONS:
Standard:
New:
% ./sage --docbuild -h
usage: sage --docbuild [OPTIONS] DOCUMENT (FORMAT | COMMAND)
Build or return information about Sage documentation. DOCUMENT name of the document to
build FORMAT document output format COMMAND document-specific command Note that DOCUMENT
may have the form 'file=/path/to/FILE', which builds the documentation for the specified
file. A DOCUMENT and either a FORMAT or a COMMAND are required, unless a list of one or
more of these is requested.
positional arguments:
document
{html,pdf,changes,htmlhelp,inventory,json,latex,linkcheck,pickle,web}
Standard:
Any idea why it's not preserving the line breaks in the help message? We could try the following changes, but I would like to understand why it's not respecting our formatting for the strings.
diff --git a/src/sage_docbuild/__init__.py b/src/sage_docbuild/__init__.py
index 1f692f1abe..84a6444ec1 100644
--- a/src/sage_docbuild/__init__.py
+++ b/src/sage_docbuild/__init__.py
@@ -1385,9 +1385,6 @@ def help_description(s="", compact=False):
character.
"""
s += "Build or return information about Sage documentation.\n\n"
- s += " DOCUMENT name of the document to build\n"
- s += " FORMAT document output format\n"
- s += " COMMAND document-specific command\n\n"
s += "Note that DOCUMENT may have the form 'file=/path/to/FILE',\n"
s += "which builds the documentation for the specified file.\n\n"
s += "A DOCUMENT and either a FORMAT or a COMMAND are required,\n"
@@ -1625,8 +1622,10 @@ def setup_parser():
help="if ARG is 'reference', list all subdocuments"
" of en/reference. If ARG is 'all', list all main"
" documents")
- parser.add_argument("document", nargs='?', type=str)
- parser.add_argument("format", nargs='?', type=str, choices=get_formats())
+ parser.add_argument("document", nargs='?', type=str,
+ help='name of the document to build')
+ parser.add_argument("format", nargs='?', type=str, choices=get_formats(),
+ help='document output format')
return parser
@@ -1722,7 +1721,8 @@ def main():
# trying to build.
name, typ = args.document, args.format
if not name or not type:
- raise ValueError('both document and format should be given')
+ parser.print_help()
+ sys.exit(1)
# Set up module-wide logging.
setup_logger(args.verbose, args.color)Branch pushed to git repo; I updated commit sha1. New commits:
00d94ff | fix for empty input |
Branch pushed to git repo; I updated commit sha1. New commits:
b02c912 | fine tuning details in argparse for docbuild |
here is a new tentative
Looks good. I have a few more proposed changes: two of the "examples" in help_examples don't work, so I am fixing one and changing one. Also, ./sage --docbuild -H used to print a longer message, and I am restoring that. Let me know if you object to any of these changes, of course.
Branch pushed to git repo; I updated commit sha1. New commits:
c6e7bda | trac 31366: minor fixes to help_examples |
A bit more explanation: ./sage -FDC all used to print formats, documents, and commands, but now it just prints formats and exits. Also, the -S option requires -S=-OPTS because of the hyphen in OPTS; this appears to be a known issue with argparse: https://stackoverflow.com/questions/16174992/cant-get-argparse-to-read-quoted-string-with-dashes-in-it. Finally, ./sage --docbuild developer -j html is not good: the document and format should come first, the -j just confuses things (and isn't necessary anyway, since mathjax has been the default forever).
Ready for review? I am happy with your changes.
Reviewer: John Palmieri
I would say so. I have forgotten if there were remaining issues or not.
Dependencies: #32946
Okay, let's merge it. (I added #32946 as a dependency because otherwise there would be a merge conflict.)
Changed branch from public/ticket/31366 to 0b62c16