Sphinx warning: 'Could not parse cython argspec'

[mpatel]

Sphinx warnings from building the HTML reference manual include:

matrix/matrix_integer_dense.rst:6: (WARNING/2) error while formatting signature for sage.matrix.matrix_integer_dense.Matrix_integer_dense.LLL: Could not parse cython argspec
plot/plot3d/base.rst:6: (WARNING/2) error while formatting signature for sage.plot.plot3d.base.Graphics3d.export_jmol: Could not parse cython argspec

Related: #8244.

[mpatel]

Alternate way to get Cython argspec. sage repo.

[mpatel]

The patch should work for the cases in the description.

[jhpalmieri]

Should patches like this also come with a corresponding one to sagenb.misc.sageinspect, as in #8324?

Looks good, no warnings when building the reference manual for these two files, and the relevant pages look good. Nice solution.

[jhpalmieri]

(Is it dangerous to use "exec" here? It looks okay to me, but I feel as though I should always be suspicious of using it.)

[mpatel]

Replying to jhpalmieri:

(Is it dangerous to use "exec" here? It looks okay to me, but I feel as though I should always be suspicious of using it.)

I think you're right. The exec'd code could have bad side effects. I'm about to attach a patch that uses this AST code, instead.

[mpatel]

AST version. sage repo.

[mpatel]

I've attached the AST version, which seems to work for me, although I have no formal training in computer science. If the patch looks good, I'll add a sagenb repository patch.

[jhpalmieri]

On one hand, this seems to fix the two particular doctests in question. On the other, it's not perfect. I can see two problems, one of which I know how to fix:

• if the source has the (unlikely) form def f({(1,2,3): True}): ..., then this version (and all previous versions) will think that the arg spec ends after (1,2,3):. The function _sage_getargspec_from_ast can actually handle this kind of thing, though, so I think we should pass the entire source code to it, rather than truncate at the first ):. That is, delete line 470 and change line 471 from

  proxy = 'def dummy' + source[beg:end] + '\n    return' 
  

  to

  proxy = 'def dummy' + source[beg:] + '\n    return' 
  

• if the docstring has type information, it can't handle it. I don't know what to do about this, or if it's worth it.

Should we fix the first problem and then go ahead with this? I also notice that the methods for SageArgSpecVisitor don't have doctests. Is that possible for these sorts of things? I know nothing about ast.

[mpatel]

Another clause. More doctests. Apply only this patch. sage repo.

[mpatel]

V3:

• Implements your idea to pass the entire source code. I've included the previous version as a fallback, since LLL has a Cython-only construct:

  R = <Matrix_integer_dense>self.new_matrix(entries=map(ZZ,A.list()))
  

• Has extra, more direct doctests of SageArgSpecVisitor's methods.

I don't mind leaving the second problem for another day.

[jhpalmieri]

Looks good. All doctests pass, fixes the two problems.

[mpatel]

For SageNB's sageinspect.py. sagenb repo.

[mpatel]

I've attached a version for SageNB.

[jhpalmieri]

Unless I'm misunderstanding, the sagenb patch needs work: all of the doctests in the patch import from sage.misc.sageinspect rather than from sagenb....

[mpatel]

Fix doctest imports. Replaces previous sagenb patch.

[mpatel]

Oops. Sorry about that. V2 is up.

[mhansen]

I've merged the Sage library patch. When this is merged in the notebook, we can close this ticket.

[mpatel]

Merged sagenb patch V2 in sagenb-0.7.5.3, which will be at #8435.