Opened 11 years ago
Closed 11 years ago
#8244 closed defect (fixed)
Annoying warnings when building the HTML reference manual
Reported by: | mpatel | Owned by: | mvngu |
---|---|---|---|
Priority: | major | Milestone: | sage-4.3.4 |
Component: | documentation | Keywords: | |
Cc: | mvngu, jhpalmieri, mhansen, craigcitro, robertwb | Merged in: | sage-4.3.4.alpha0 |
Authors: | John Palmieri, Mitesh Patel | Reviewers: | Mitesh Patel, John Palmieri |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Let's fix or suppress these, so that it's easier to identify new problems.
See the attachment.
Attachments (5)
Change History (25)
Changed 11 years ago by
comment:1 Changed 11 years ago by
Should we follow #6419 for the nested classes? We could use that approach for
sagenb.notebook.twist.UserToplevel.userchild_download_worksheets.zip
comment:2 Changed 11 years ago by
- Cc mhansen craigcitro added
I've looked at these a bit, but I have no idea how to fix them. Oh, except for
plot/plot3d/base.rst:6: (WARNING/2) error while formatting signature for sage.plot.plot3d.base.Graphics3d.export_jmol: Could not parse cython argspec
This is because _sage_getargspec_cython
in sage.misc.sageinspect is a little broken, and fixing it might be lot of work: right now it parses arguments to Cython function by doing basic string and regular expression manipulations, in particular separating arguments by splitting the line on commas. The function export_jmol
has argspec
def export_jmol(self, filename='jmol_shape.jmol', force_reload=False, zoom=100, spin=False, background=(1,1,1), stereo=False, mesh=False, dots=False, perspective_depth = True, orientation = (-764,-346,-545,76.39), **ignored_kwds):
and so splitting at commas doesn't work. To do this right, we either need to write a good parser (which deals with nested lists, tuples, etc.), or perhaps change the whole way we deal with argspecs for Cython functions; for example, just read off the entire string of all the arguments and pass that to the appropriate function, instead of trying to break it into arguments, default values, etc. This is a pretty rare problem and it looks annoying to deal with, so I haven't felt like putting any time into it.
mhansen and craigcitro: any comments or ideas?
comment:3 Changed 11 years ago by
Ticket #8243 is a duplicate of this one.
comment:4 Changed 11 years ago by
- Cc robertwb added
Well, Cython itself has to parse such a thing, but I don't know how easy it is go get the data we need out of Cython. Maybe Robert might have an idea?
comment:5 Changed 11 years ago by
I'm not very familiar with ASTs (or ast), but with
import ast import inspect import sage.misc.sageinspect as sms class SageVisitor(ast.NodeVisitor): def visit_Name(self, node): what = node.id if what == 'None': return None elif what == 'True': return True elif what == 'False': return False return node.id def visit_Num(self, node): return node.n def visit_Str(self, node): return node.s def visit_List(self, node): t = [] for n in node.elts: t.append(self.visit(n)) return t def visit_Tuple(self, node): t = [] for n in node.elts: t.append(self.visit(n)) return tuple(t) def visit_Dict(self, node): d = {} for k, v in zip(node.keys, node.values): d[self.visit(k)] = self.visit(v) return d def getargspec_via_ast(source): if not isinstance(source, basestring): source = sms.sage_getsource(source) ast_args = ast.parse(source.lstrip()).body[0].args args = [] defaults = [] for a in ast_args.args: args.append(SageVisitor().visit(a)) for d in ast_args.defaults: defaults.append(SageVisitor().visit(d)) return inspect.ArgSpec(args, ast_args.vararg, ast_args.kwarg, tuple(defaults) if defaults else None)
I get
sage: inspect.getargspec(factor) == getargspec_via_ast(factor) True sage: getargspec_via_ast(sage.plot.plot3d.base.Graphics3d.export_jmol) ArgSpec(args=['self', 'filename', 'force_reload', 'zoom', 'spin', 'background', 'stereo', 'mesh', 'dots', 'perspective_depth', 'orientation'], varargs=None, keywords='ignored_kwds', defaults=('jmol_shape.jmol', False, 100, False, (1, 1, 1), False, False, False, True, (-764, -346, -545, 76.390000000000001)))
Maybe we can use this as a last resort? I think we'd first need to remove Cython-specific constructs.
comment:6 follow-up: ↓ 8 Changed 11 years ago by
It seems that nearly all of the warnings are from Cython files and that we really need a more robust analogue of inspect.getargspec
for Cython source.
For the ElementMethods
, ParentMethods
, userchild_download_worksheets.zip
warnings, we can expand the autodoc skip method handler.
The other warnings seem to be reST formatting problems, but I'm not sure about
<autodoc>:0: (ERROR/3) Unexpected indentation. <autodoc>:0: (ERROR/3) Unexpected indentation.
comment:7 Changed 11 years ago by
Can autodoc handle nested classes?
comment:8 in reply to: ↑ 6 Changed 11 years ago by
Replying to mpatel:
It seems that nearly all of the warnings are from Cython files and that we really need a more robust analogue of
inspect.getargspec
for Cython source.
Or for now, a way to suppress all of the error message.
I'm not sure about the <autodoc>...
errors, either.
comment:9 Changed 11 years ago by
The following seems to remove the "arg is not a Python function" warnings:
-
autodoc.py
old new class MethodDocumenter(ClassLevelDocumen 1005 1005 else: 1006 1006 return None 1007 1007 else: 1008 argspec = inspect.getargspec(self.object) 1008 # The check above misses ordinary Python methods in Cython 1009 # files. 1010 try: 1011 argspec = inspect.getargspec(self.object) 1012 except TypeError: 1013 if (inspect.ismethod(self.object) and 1014 self.env.config.autodoc_builtin_argspec): 1015 argspec = self.env.config.autodoc_builtin_argspec(self.object.im_func) 1016 else: 1017 return None 1009 1018 if argspec[0] and argspec[0][0] in ('cls', 'self'): 1010 1019 del argspec[0][0] 1011 1020 return inspect.formatargspec(*argspec)
Should we copy autodoc.py
to SAGE_DOC/common/sage_autodoc.py
, make all of our changes in the latter, and add the custom extension in SAGE_DOC/common/conf.py
?
comment:10 Changed 11 years ago by
The attached patch should remove the warnings that end in
.next: arg is not a module, class, method, function, traceback, frame, or code object
comment:11 Changed 11 years ago by
With the patches, I get about 10 non-reST warnings. I think we can skip
sagenb.notebook.twist.UserToplevel.userchild_download_worksheets.zip
in conf.py
. We should coordinate the spkg updates.
comment:12 Changed 11 years ago by
This is great progress. By the way, I know how to fix the warnings for sage.misc.sagedoc.process_dollars
: we shouldn't run process_dollars
on this function. That is, change the first line of process_dollars
in conf.py from
if len(docstringlines) > 0:
to
if len(docstringlines) > 0 and name.find("process_dollars") == -1:
Similarly for process_mathtt
. I've included this change in my patch.
Should we copy autodoc.py to SAGE_DOC/common/sage_autodoc.py, make all of our changes in the latter, and add the custom extension in SAGE_DOC/common/conf.py?
This is an interesting idea since we're patching it so much. Then it would be under Sage revision control. We would just need to keep an eye on it whenever we upgrade Sphinx. I'm posting a patch which adds it. Actually, it doesn't add the whole thing, since I got an error when I tried that. Instead, it does
from sphinx.ext.autodoc import *
and then it defines FunctionDocumenter
, MethodDocumenter
, setup
, and ClassDocumenter
(this one isn't patched right now, but it looks like it will be in #7448).
comment:13 Changed 11 years ago by
My patch also skips sagenb.notebook.twist.UserToplevel.userchild_download_worksheets.zip
.
comment:14 Changed 11 years ago by
Here's one more patch: this fixes one last warning message about sage.misc.process_dollars.
comment:15 Changed 11 years ago by
- Reviewers set to Mitesh Patel
Thanks! Thanks also for making the extension patch. To the extent it counts, my review is positive.
I'll rebase #7448.
To the release manager: Please apply
trac_8244-slot_wrapper_argspec.patch trac_8244-conf-autodoc.patch trac_8244-sagedoc.patch
comment:16 Changed 11 years ago by
- Reviewers changed from Mitesh Patel to Mitesh Patel, John Palmieri
- Status changed from new to needs_review
comment:17 Changed 11 years ago by
- Status changed from needs_review to positive_review
comment:18 follow-up: ↓ 19 Changed 11 years ago by
I've attached a replacement for the "conf-autodoc" patch that adds all of autodoc
(as sage_autodoc
) and, with some redefinition, avoids the ExtensionError
.
A self-contained sage_autodoc
should make it less likely that sage_autodoc
stops working, when we upgrade or test new versions of Sphinx. (I was just hit by this during experiments with a development version).
What do you think?
comment:19 in reply to: ↑ 18 Changed 11 years ago by
Replying to mpatel:
I've attached a replacement for the "conf-autodoc" patch that adds all of
autodoc
(assage_autodoc
) and, with some redefinition, avoids theExtensionError
.A self-contained
sage_autodoc
should make it less likely thatsage_autodoc
stops working, when we upgrade or test new versions of Sphinx. (I was just hit by this during experiments with a development version).What do you think?
Looks good to me. Still a positive review.
comment:20 Changed 11 years ago by
- Merged in set to sage-4.3.4.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
Merged in this order:
HTML reference manual docbuild warnings for 4.3.3.alpha0. Not a patch.