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

Sphinx should be aware of all.py to find its links #9128

Closed
hivert opened this issue Jun 3, 2010 · 78 comments
Closed

Sphinx should be aware of all.py to find its links #9128

hivert opened this issue Jun 3, 2010 · 78 comments

Comments

@hivert
Copy link

hivert commented Jun 3, 2010

Though sphinx is perfectly working with target in the local module he isn't
able to find reference target from other modules even if they are exported in
all.py. For example, if I want to link Parent from anywhere but parent.pyx, I
have to write the full path (ie. :class:`~sage.structure.parent.Parent`)
even if it is imported in my module. I find this extremely annoying since, in
the task of improving the category doc, I'm setting up a lot of huge cross
references such as:

    :meth:`Algebras.ParentMethods.algebra_generators()
    <sage.categories.algebras.Algebras.ParentMethods.algebra_generators>`.

I would be very happy if I had to write only

    :meth:`Algebras.ParentMethods.algebra_generators`

The following patch should solve this issue

I set up intersphinx so that links to Python's doc are correctly
resolved. The patch
attachment: trac_9128-intersphinx_python_database-fh.patch contains Python's
crossref database downloaded from http://docs.python.org/objects.inv I'm also
using intersphinx to solve links to the reference manual from the other
documents.

New option for docbuild

I added the option --warn-links to the documentation build command as in

    sage -docbuild --warn-links reference htm

When the option is used, Sphinx will issue a warning, whenever a link is not resolved.

Extra features

Moreover, I add a reference target to every automatically created .rst
file associated to python source files. It can by used to set-up a link toward
the file and thus get the title rather than the module. For example
:ref:sage.categories.primer setup a link to the help page with title
"Elements, parents, and categories in Sage: a (draft of) primer" rather
than
sage.categories.primer which you get with the link
:mod:sage.categories.primer.

Apply both:

Note: order is irrelevant

Depends on #11251
Depends on #12490
Depends on #12572

CC: @nthiery @nexttime @novoselt @mwhansen

Component: documentation

Keywords: Sphinx links

Author: Florent Hivert

Reviewer: Andrey Novoseltsev, Nicolas M. Thiéry

Merged: sage-5.0.beta8

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

@hivert
Copy link
Author

hivert commented Jun 3, 2010

comment:1

The patch here is a prototype, not yet ready for review. Comments or suggestions are mostly welcome.

@hivert hivert assigned hivert and unassigned sagetrac-mvngu Jun 3, 2010
@hivert

This comment has been minimized.

@hivert
Copy link
Author

hivert commented Jun 3, 2010

comment:4

The submitted patch seems to behave as I want. So I put needs review. However if I receive some good advice on sage-devel or sphinx-users I may still change it. Anyway, Please comment.

@novoselt
Copy link
Member

novoselt commented Jun 7, 2010

comment:6

I have just upgraded an installation of sage-4.4.2 to 4.4.3, applied this patch, set SAGE_DOC_WARN_DANGLING_LINKS to 1, and then got the following error:

novoselt@sage:/scratch/novoselt/sage/devel/sage-main$ ../../sage -docbuild reference html
sphinx-build -b html -d /mnt/usb1/scratch/novoselt/sage/devel/sage/doc/output/doctrees/en/reference    /mnt/usb1/scratch/novoselt/sage/devel/sage/doc/en/reference /mnt/usb1/scratch/novoselt/sage/devel/sage/doc/output/html/en/reference
Running Sphinx v0.6.3
loading pickled environment... done
building [html]: targets for 798 source files that are out of date
updating environment: [config changed] 802 added, 0 changed, 0 removed
reading sources... [100%] structure
/mnt/usb1/scratch/novoselt/sage/local/lib/python2.6/site-packages/sage/schemes/elliptic_curves/sha_tate.py:docstring of sage.schemes.elliptic_curves.sha_tate.Sha.bound_kato:12: (WARNING/2) Definition list ends without a blank line; unexpected unindent.
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [  0%] coercion
Exception occurred:
  File "/mnt/usb1/scratch/novoselt/sage/devel/sage/doc/common/conf.py", line 444, in find_sage_dangling_links
    fromdocname = getattr(sys.modules[modname], basename).__module__
KeyError: None
The full traceback has been saved in /tmp/sphinx-err-Unu279.log, if you want to report the issue to the author.
Please also report this if it was a user error, so that a better error message can be provided next time.
Send reports to sphinx-dev@googlegroups.com. Thanks!
Build finished.  The built documents can be found in /mnt/usb1/scratch/novoselt/sage/devel/sage/doc/output/html/en/reference
novoselt@sage:/scratch/novoselt/sage/devel/sage-main$

Then I removed this patch, built documentation successfully, pushed this and some other patches and now it seems to work fine. Perhaps it was an unreproducible glitch unrelated to the patch. In general it seems to work as expected and showed me a couple of mistakes in my modules.

I hesitate to switch status to positive review because of the error above and because I don't really understand the code, but I think that this is a great addition and will use this patch from now on!

@novoselt
Copy link
Member

novoselt commented Jun 7, 2010

comment:7

I tried the same thing on my own computer - upgraded from 4.4.2 to 4.4.3, applied this patch and tried to build documentation (without setting the environment variable this time). Same error repeatedly, but after popping the patch out everything goes OK. So something has to be done ;-)

@hivert
Copy link
Author

hivert commented Jun 7, 2010

comment:8

Hi Andrey

Replying to @novoselt:

I tried the same thing on my own computer - upgraded from 4.4.2 to 4.4.3, applied this patch and tried to build documentation (without setting the environment variable this time). Same error repeatedly, but after popping the patch out everything goes OK. So something has to be done ;-)

Thanks for looking at that. My patch was working for .py and .pyx
file but not .rst file. The new updated file should work. I didn't get
any comment from #sphinx-devel except that missing-reference is
the good entry point. So except if some expert pop up and suggest some more
improvements, I consider this proposal as a good one. Please review.

@novoselt
Copy link
Member

novoselt commented Jun 7, 2010

comment:9

Now I upgraded from 4.4.3 to 4.4.4.alpha0. The new patch works better, but not perfect:

WARNING: undefined symbol :meth:`dual` in module sage.categories.dual
writing output... [ 13%] sage/categories/examples/algebras_with_basis
Exception occurred:
  File "/mnt/usb1/scratch/novoselt/sage/devel/sage/doc/common/conf.py", line 453, in find_sage_dangling_links
    current_module = sys.modules[modname]
KeyError: u'sage.categories.examples.algebras_with_basis'
The full traceback has been saved in /tmp/sphinx-err-rFQQUv.log, if you want to report the issue to the author.
Please also report this if it was a user error, so that a better error message can be provided next time.
Send reports to sphinx-dev@googlegroups.com. Thanks!
Build finished.  The built documents can be found in /mnt/usb1/scratch/novoselt/sage/devel/sage/doc/output/html/en/reference

@hivert
Copy link
Author

hivert commented Jun 7, 2010

comment:10

Replying to @novoselt:

Now I upgraded from 4.4.3 to 4.4.4.alpha0. The new patch works better, but not perfect:

Hum ! I know how to workaround those problems but I don't understand why this is happening ! Unfortunately, I don't want to upgrade and I can't reproduce the bug... Can you try to remove the directory $SAGE_ROOT/devel/sage/doc/output and relaunch the compilation. Does it still bug ?

Florent

@novoselt
Copy link
Member

novoselt commented Jun 7, 2010

comment:11

Successful built with 552 warnings!

@novoselt
Copy link
Member

novoselt commented Jun 8, 2010

comment:12

While working on my patches, I am getting the following from this one:

WARNING: undefined symbol :meth:`sage.geometry.fan.RationalPolyhedralFan._compute_cone_lattice` in module sage.geometry.cone 

I don't understand what is wrong. There are no typos and this module does exist in my installation. Is it because I don't import this class/module? Or because it is an underscore method and so there is no record of it in the documentation? (In this case, I actually make a reference to the source code of this method, so I just want the name to be typeset in the same way as other methods, I don't care that the link will not work.)

@hivert
Copy link
Author

hivert commented Jun 8, 2010

comment:13

Replying to @novoselt:

While working on my patches, I am getting the following from this one:

WARNING: undefined symbol :meth:`sage.geometry.fan.RationalPolyhedralFan._compute_cone_lattice` in module sage.geometry.cone 

I don't understand what is wrong. There are no typos and this module does exist in my installation. Is it because I don't import this class/module? Or because it is an underscore method and so there is no record of it in the documentation? (In this case, I actually make a reference to the source code of this method, so I just want the name to be typeset in the same way as other methods, I don't care that the link will not work.)

Hi Andrey,

Thanks for beta testing my patch ! It this module included in the documentation ? More precisely is there a link in some .rst file (probably doc/en/reference/geometry.rst) ? Because if it is not imported nor linked from the doc, Sphinx as no way to find it but importing the module. This is something I tried to avoid for performance reason... If you are still having some problem, can you please make your patch accessible so that I can debug with it. I don't care if the code is not working...

You probably already figured that out, but I must confess that I put this patch here for comment but it is clear that it has not sufficiently been shaken. So many thanks for helping me on that and sorry to cause some trouble.

@novoselt
Copy link
Member

novoselt commented Jun 8, 2010

comment:14

Hi Florent!

I have figured out that the problem is with the underscore - if I use a "common" method from the same non-imported patch, link is determined just fine.

It may be a good idea not to give warnings in such cases, but on the other hand it is probably quite rare and there is no need to make logic of this patch more convoluted. (In my case the docstring of one of the functions says "see the source code of ... for more involved examples", since those examples cannot be easily written as standalone code and I didn't want to write something "involved, but artificial.")

Since I really like this functionality, I will continue using your patch (and its fresh versions if they become available) and report new problems, if there will be any. But for the final review we will need someone else, since I don't know how Sphinx works and cannot comment on the code itself.

Thank you!
Andrey

@hivert
Copy link
Author

hivert commented Jun 11, 2010

comment:15

It may be a good idea not to give warnings in such cases, but on the other hand it is probably quite rare and there is no need to make logic of this patch more convoluted. (In my case the docstring of one of the functions says "see the source code of ... for more involved examples", since those examples cannot be easily written as standalone code and I didn't want to write something "involved, but artificial.")

Can you describe more precisely what's happening ? Maybe with a very small example... I'm not sure to understand what you are doing... I have the impression that you are requesting me to remove warnings about private methods (i.e.: starting with underscore).

Since I really like this functionality, I will continue using your patch (and its fresh versions if they become available) and report new problems, if there will be any. But for the final review we will need someone else, since I don't know how Sphinx works and cannot comment on the code itself.

I'll try to ping some on sage-devel.

@novoselt
Copy link
Member

comment:16

I think that I want to have a distinction between "totally wrong names" and names which were successfully found in the Sage library (therefore, it makes sense to reference them), but do not have corresponding entries in the documentation files (therefore, it is not possible to convert them into a working hyperlink). Private/underscore methods are one example (I would like actually to seem them in the documentation "on demand", but maybe there are arguments against it), another is reference to objects in modules which are not included into documentation (maybe there will be no such modules eventually). I hope this is more clear, but in any way it is a small point.

@hivert
Copy link
Author

hivert commented Jun 11, 2010

comment:17

Replying to @novoselt:

I think that I want to have a distinction between "totally wrong names" and names which were successfully found in the Sage library (therefore, it makes sense to reference them), but do not have corresponding entries in the documentation files (therefore, it is not possible to convert them into a working hyperlink). Private/underscore methods are one example (I would like actually to seem them in the documentation "on demand", but maybe there are arguments against it), another is reference to objects in modules which are not included into documentation (maybe there will be no such modules eventually). I hope this is more clear, but in any way it is a small point.

This should be exactly what I'm doing: I issue two different kinds of warning:

  • undefined symbol :%s:`%s` in %s
  • symbol :%s:`%s` linked from %s is defined in %s but not documented

Bu maybe sometime I fail finding a symbol and therefore issue the first warning instead of the second one... Is this happening for you ?

Florent

@novoselt
Copy link
Member

comment:18

I get the first kind of warning and now the class is actually imported (although in the end of the module to avoid circular imports).

I have just uploaded a fresh patch to #8987 (so fresh, that it is actually very raw ;-)) if you want to reproduce the issue. Beware that it is a chain of patches, I think it is possible to apply only those listed in #9062, #8986, and #8987 (especially if you are working with 4.4.4.alpha0, where all prerequisites but one are merged).

@jdemeyer
Copy link

comment:57

Replying to @hivert:

export extra_mem_bot=8000000; tex $*

Jeroen: will you be so kind to try if this works ?

This sort of works. It builds the manual, but here's the strange thing: every run of pdflatex consumes more and more memory. That is, if I run pdflatex 10 times in a row on reference.tex with that extra environment variable, I will certainly hit the error again.

@jdemeyer
Copy link

Changed dependencies from #11251, #12490 to #11251, #12490, #12572

@jdemeyer
Copy link

comment:58

With extra_mem_top=2000000 (note bot vs. top), it seems to work as it should:

Here is how much of TeX's memory you used:
 57776 strings out of 94101
 2165001 string characters out of 3915810
 1541607 words of memory out of 2966904
 33568 multiletter control sequences out of 10000+50000
 99604 words of font info for 164 fonts, out of 1200000 for 2000
 638 hyphenation exceptions out of 8191
 38i,25n,51p,6802b,946s stack positions out of 5000i,500n,6000p,200000b,50000s

See #12572.

@jdemeyer
Copy link

comment:59

The new Sphinx spkg to add extra memory to latex is ready for review at #12572.

@jdemeyer
Copy link

comment:60

The files

doc/common/python.inv
doc/common/update-python-inv.sh

should be put in SAGE_ROOT/devel/sage/MANIFEST.in

@jdemeyer
Copy link

jdemeyer commented Mar 1, 2012

Work Issues: MANIFEST.in

@hivert

This comment has been minimized.

@hivert
Copy link
Author

hivert commented Mar 4, 2012

comment:62

Attachment: trac_9128-MANIFEST-fh.patch.gz

Replying to @jdemeyer:

The files

doc/common/python.inv
doc/common/update-python-inv.sh

should be put in SAGE_ROOT/devel/sage/MANIFEST.in

Thanks for pointing this out. I just added a patch attachment: trac_9128-MANIFEST-fh.patch which should do that. Please double check as I don't really know what should be there.

Florent

@nthiery
Copy link
Contributor

nthiery commented Mar 6, 2012

comment:63

This looks good. Back to positive review!

@jdemeyer
Copy link

jdemeyer commented Mar 7, 2012

Changed work issues from MANIFEST.in to none

@jdemeyer
Copy link

Merged: sage-5.0.beta8

@nthiery
Copy link
Contributor

nthiery commented Mar 13, 2012

comment:66

Yippee, a good thing done!

Thanks Florent for all the energy you put into this ticket :-)
Thanks everyone for the review and support.

@jdemeyer
Copy link

comment:67

See #12849 for a blocker follow-up: The argspecs of extension function/methods is broken in the Sphinx documentation.

@sagetrac-bober
Copy link
Mannequin

sagetrac-bober mannequin commented May 29, 2012

comment:68

See #13057 for a speed regression followup. It seems that this ticket slowed down introspection quite a bit.

@kini
Copy link
Collaborator

kini commented May 31, 2012

comment:69

This ticket also introduced a memory leak - 56 MB per docstring lookup. See #13057 and sage-devel.

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

6 participants