Skip to content
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

The argspecs of extension function/methods is broken in the Sphinx documentation #12849

Closed
hivert opened this issue Apr 17, 2012 · 24 comments
Closed

Comments

@hivert
Copy link

hivert commented Apr 17, 2012

In the current Sphinx HTML doc, the extenstion function and methods have no arguments setup.

See for example the documentation of

reference/sage/symbolic/expression.html#sage.symbolic.expression.Expression.N

which was ok (see there) in Sage 4.8. The problem was introduced in sage-5.0.beta8.

We should add a regression test on that kinds of problem.

Apply:

Component: documentation

Keywords: argspecs Cython

Author: Florent Hivert, Jeroen Demeyer

Reviewer: Mike Hansen

Merged: sage-5.0.beta14

Issue created by migration from https://trac.sagemath.org/ticket/12849

@jdemeyer
Copy link

Doctest for the issue

@jdemeyer
Copy link

comment:2

Attachment: 12849_doctest.patch.gz

I created a doctest for this issue, which obviously fails on recent Sage betas.

@hivert
Copy link
Author

hivert commented Apr 18, 2012

comment:3

Replying to @jdemeyer:

I created a doctest for this issue, which obviously fails on recent Sage betas.

Thanks ! I tried to figureout a way to call directly Sphinx but this is much easier. I'll try to work on this this afternoon. Do you have somewhere the various beta compiled so that I can rsync them on boxen to help bissecting ? So far the few guesses I made to find the culprit were wrong.

@jdemeyer
Copy link

comment:4

sage-5.0.beta5 is still okay.

@jdemeyer
Copy link

comment:5

Replying to @hivert:

Replying to @jdemeyer:

I created a doctest for this issue, which obviously fails on recent Sage betas.

Thanks ! I tried to figureout a way to call directly Sphinx but this is much easier. I'll try to work on this this afternoon. Do you have somewhere the various beta compiled so that I can rsync them on boxen to help bissecting ? So far the few guesses I made to find the culprit were wrong.

Look at http://boxen.math.washington.edu/home/release/. There should be binaries for all Sage betas, made on sage.math or boxen.math.

@jdemeyer

This comment has been minimized.

@hivert
Copy link
Author

hivert commented Apr 18, 2012

comment:7

It seems that I have a fix, but I don't understand how it worked before ! Still looking.

Florent

@jdemeyer
Copy link

comment:8

It looks like this was introduced in sage-5.0.beta8.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

comment:9

The culprit is #9128.

@hivert
Copy link
Author

hivert commented Apr 18, 2012

comment:10

Thanks for investigating.

Replying to @jdemeyer:

The culprit is #9128.

Strange ! It was an obvious candidate and I'm pretty sure I started by checking this one. Still I don't understand what could cause that in #9128. I'm testing my fix.

Florent

@hivert
Copy link
Author

hivert commented Apr 19, 2012

comment:12

Hi,

I got the fix ! I'm definitely the culprit. For strange reason the following lines were removed by #9128. Putting them back should fix the problem.

diff --git a/doc/common/conf.py b/doc/common/conf.py
--- a/doc/common/conf.py
+++ b/doc/common/conf.py
@@ -576,6 +576,8 @@ def find_sage_dangling_links(app, env, n
     newnode.append(contnode)
     return newnode
 
+from sage.misc.sageinspect import sage_getargspec
+autodoc_builtin_argspec = sage_getargspec
 
 def setup(app):
     app.connect('autodoc-process-docstring', process_docstring_cython)

@hivert
Copy link
Author

hivert commented Apr 19, 2012

Author: Florent Hivert, Jeroen Demeyer

@hivert
Copy link
Author

hivert commented Apr 19, 2012

comment:13

Attachment: trac_12849-extfunc_argspec_html_fix-fh.patch.gz

@hivert

This comment has been minimized.

@hivert
Copy link
Author

hivert commented Apr 19, 2012

comment:14

Jeroen: I'm reviewing your patch. Can you review mine ?

Florent

@jdemeyer
Copy link

comment:15

Replying to @hivert:

Can you review mine ?

Sorry, no. I'm not at all familiar with the docbuilding process.

@mwhansen
Copy link
Contributor

comment:16

Florent's patch is good, as is Jeroen (even if it is a bit sensitive to the particular HTML output used by Sphinx).

@mwhansen
Copy link
Contributor

Reviewer: Mike Hansen

@hivert
Copy link
Author

hivert commented Apr 20, 2012

comment:17

Replying to @mwhansen:

Florent's patch is good, as is Jeroen (even if it is a bit sensitive to the particular HTML output used by Sphinx).

Thanks Mike !

@jdemeyer
Copy link

Merged: sage-5.0.beta14

@hughrthomas
Copy link

comment:19

Hi!

The doctest introduced by this patch, at line 22 of sage/misc/sagedoc.py, is causing the sagepad.org patchbot to fail on some (trivial, unrelated) patches. The problem seems to be that the doctest is trying to check the documentation when the documentation isn't built.

I don't know if this is really a problem with this patch, or with the patchbot, but I thought I would try reporting it here. If I should try elsewhere, please let me know!

You can see this failure at #12943, which is a completely trivial patch. (It's also making the sagepad.org patchbot fail on #10527. If you look at that ticket, though, don't look at the log for Volker's patchbot, which is having a different problem.)

cheers,

Hugh

@jhpalmieri
Copy link
Member

comment:20

A complete set of documentation is an integral part of a Sage build, so doctests are supposed to fail if the documentation hasn't been built. This is not because of this ticket, I don't think.

@hughrthomas
Copy link

comment:21

Thanks. The issue seems to be with the particular patchbot -- I have posted to sage-devel about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants