Opened 16 months ago
Closed 5 months ago
#31366 closed enhancement (fixed)
docbuild: switch from optparse to argparse
Reported by: | jhpalmieri | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.5 |
Component: | documentation | Keywords: | |
Cc: | Merged in: | ||
Authors: | Frédéric Chapoton | Reviewers: | John Palmieri |
Report Upstream: | N/A | Work issues: | |
Branch: | 0b62c16 (Commits, GitHub, GitLab) | Commit: | 0b62c166044271f0b7db4f42bb174e560562a364 |
Dependencies: | #32946 | Stopgaps: |
Description (last modified by )
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
Change History (37)
comment:1 Changed 15 months ago by
- Milestone changed from sage-9.3 to sage-9.4
comment:2 Changed 10 months ago by
- Milestone changed from sage-9.4 to sage-9.5
comment:3 Changed 10 months ago by
a little bit in #32331
comment:4 Changed 10 months ago by
- Branch set to public/ticket/31366
- Commit set to 159ec4b5b77cc7b3b5a008a08d01a691ed506b3f
first tentative
New commits:
159ec4b | first step towards using argparse for doc command line arguments
|
comment:5 Changed 10 months ago by
- Commit changed from 159ec4b5b77cc7b3b5a008a08d01a691ed506b3f to 56ce7b68910f0f6a8e00c58239fa81a2bd0c1fd2
Branch pushed to git repo; I updated commit sha1. New commits:
56ce7b6 | more work on argparse for doc command line
|
comment:6 Changed 8 months ago by
- Commit changed from 56ce7b68910f0f6a8e00c58239fa81a2bd0c1fd2 to 98824cd9e4d5719358630e25305aee6dffd9c30f
Branch pushed to git repo; I updated commit sha1. New commits:
98824cd | Merge branch 'public/ticket/31366' in 9.5.b1
|
comment:7 Changed 7 months ago by
- Commit changed from 98824cd9e4d5719358630e25305aee6dffd9c30f to ec995b76d9096379648f1a78decc0569819e980a
comment:8 Changed 7 months ago by
- Status changed from new to needs_review
not quite sure that it works perfectly, but it seems so
comment:9 Changed 7 months ago by
- Status changed from needs_review to needs_work
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'
comment:10 Changed 7 months ago by
- Commit changed from ec995b76d9096379648f1a78decc0569819e980a to 3a4c572d5c7be610c632cf2ae66ba436636412c1
Branch pushed to git repo; I updated commit sha1. New commits:
3a4c572 | more work on argparse for docbuild
|
comment:11 Changed 7 months ago by
- Description modified (diff)
comment:12 Changed 7 months ago by
I'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
?
comment:13 Changed 7 months ago by
- Commit changed from 3a4c572d5c7be610c632cf2ae66ba436636412c1 to 70b6aa64b80ad7ff159fc4921a8dedbfb7096e29
Branch pushed to git repo; I updated commit sha1. New commits:
70b6aa6 | another fix for argparse in docbuild
|
comment:14 Changed 7 months ago by
here is a tentative. I had no time to check
comment:15 Changed 7 months ago by
- Commit changed from 70b6aa64b80ad7ff159fc4921a8dedbfb7096e29 to 914ce7d900553b87d59fc27e3f98ca16adfa93d2
Branch pushed to git repo; I updated commit sha1. New commits:
914ce7d | more work on argparse for docbuild
|
comment:16 Changed 7 months ago by
more fixes, but apparently not ok yet
comment:17 Changed 7 months ago by
- Commit changed from 914ce7d900553b87d59fc27e3f98ca16adfa93d2 to 199f8a03e9907b71453730e540933e6b5997d5e6
Branch pushed to git repo; I updated commit sha1. New commits:
199f8a0 | trac 31366: more fixes
|
comment:18 Changed 7 months ago by
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.
comment:19 Changed 7 months ago by
- Commit changed from 199f8a03e9907b71453730e540933e6b5997d5e6 to 84bde616d574537bf92eb77e1b7512942fe042e5
comment:20 Changed 7 months ago by
I was working on that right now. I had to merge my branch with yours, hopefully in a correct way. Starts to look better.
comment:21 Changed 7 months ago by
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
comment:22 Changed 7 months ago by
- Commit changed from 84bde616d574537bf92eb77e1b7512942fe042e5 to 8c8fc39b7cdc72658c07425991d5c5738f561899
Branch pushed to git repo; I updated commit sha1. New commits:
8c8fc39 | still working on argparse for docbuilding
|
comment:23 Changed 7 months ago by
more progress done.
comment:24 Changed 7 months ago by
and the bot+linter are now green ; still needs tests and double-check, for sure
comment:25 Changed 7 months ago by
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.
-
src/sage_docbuild/__init__.py
diff --git a/src/sage_docbuild/__init__.py b/src/sage_docbuild/__init__.py index 1f692f1abe..84a6444ec1 100644
a b def help_description(s="", compact=False): 1385 1385 character. 1386 1386 """ 1387 1387 s += "Build or return information about Sage documentation.\n\n" 1388 s += " DOCUMENT name of the document to build\n"1389 s += " FORMAT document output format\n"1390 s += " COMMAND document-specific command\n\n"1391 1388 s += "Note that DOCUMENT may have the form 'file=/path/to/FILE',\n" 1392 1389 s += "which builds the documentation for the specified file.\n\n" 1393 1390 s += "A DOCUMENT and either a FORMAT or a COMMAND are required,\n" … … def setup_parser(): 1625 1622 help="if ARG is 'reference', list all subdocuments" 1626 1623 " of en/reference. If ARG is 'all', list all main" 1627 1624 " documents") 1628 parser.add_argument("document", nargs='?', type=str) 1629 parser.add_argument("format", nargs='?', type=str, choices=get_formats()) 1625 parser.add_argument("document", nargs='?', type=str, 1626 help='name of the document to build') 1627 parser.add_argument("format", nargs='?', type=str, choices=get_formats(), 1628 help='document output format') 1630 1629 return parser 1631 1630 1632 1631 … … def main(): 1722 1721 # trying to build. 1723 1722 name, typ = args.document, args.format 1724 1723 if not name or not type: 1725 raise ValueError('both document and format should be given') 1724 parser.print_help() 1725 sys.exit(1) 1726 1726 1727 1727 # Set up module-wide logging. 1728 1728 setup_logger(args.verbose, args.color)
comment:26 Changed 7 months ago by
- Commit changed from 8c8fc39b7cdc72658c07425991d5c5738f561899 to 00d94ffb760d49b6e926be00d27a0d0da0d84cff
Branch pushed to git repo; I updated commit sha1. New commits:
00d94ff | fix for empty input
|
comment:27 Changed 7 months ago by
- Commit changed from 00d94ffb760d49b6e926be00d27a0d0da0d84cff to b02c912a2181b385f4bf0413599da9eb659a48b3
Branch pushed to git repo; I updated commit sha1. New commits:
b02c912 | fine tuning details in argparse for docbuild
|
comment:28 Changed 7 months ago by
here is a new tentative
comment:29 Changed 7 months ago by
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.
comment:30 Changed 7 months ago by
- Commit changed from b02c912a2181b385f4bf0413599da9eb659a48b3 to c6e7bdaee8f214410e321fae87a8a0279ae2c7b8
Branch pushed to git repo; I updated commit sha1. New commits:
c6e7bda | trac 31366: minor fixes to help_examples
|
comment:31 Changed 7 months ago by
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).
comment:32 Changed 6 months ago by
Ready for review? I am happy with your changes.
comment:33 Changed 6 months ago by
- Reviewers set to John Palmieri
comment:34 Changed 6 months ago by
- Status changed from needs_work to needs_review
I would say so. I have forgotten if there were remaining issues or not.
comment:35 Changed 6 months ago by
- Commit changed from c6e7bdaee8f214410e321fae87a8a0279ae2c7b8 to 0b62c166044271f0b7db4f42bb174e560562a364
comment:36 Changed 6 months ago by
- Dependencies set to #32946
- Status changed from needs_review to positive_review
Okay, let's merge it. (I added #32946 as a dependency because otherwise there would be a merge conflict.)
comment:37 Changed 5 months ago by
- Branch changed from public/ticket/31366 to 0b62c166044271f0b7db4f42bb174e560562a364
- Resolution set to fixed
- Status changed from positive_review to closed
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.