Opened 9 years ago

Last modified 7 years ago

#9128 closed enhancement

Sphinx should be aware of all.py to find its links — at Version 29

Reported by: hivert Owned by: hivert
Priority: major Milestone: sage-5.0
Component: documentation Keywords: Sphinx links
Cc: nthiery, leif, novoselt, mhansen Merged in:
Authors: Florent Hivert Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by hivert)

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 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.

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. For example :ref:sage.combinat.partition setup a link to the help page.

I finally setup the extension extlinks to allows links to trac as in :trac:`9128`.

Apply both:

Change History (29)

comment:1 Changed 9 years ago by hivert

  • Owner changed from mvngu to hivert

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

comment:2 Changed 9 years ago by hivert

  • Status changed from new to needs_work

comment:3 Changed 9 years ago by leif

  • Cc leif added

comment:4 Changed 9 years ago by hivert

  • Description modified (diff)
  • Status changed from needs_work to needs_review

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.

comment:5 Changed 9 years ago by novoselt

  • Cc novoselt added

comment:6 Changed 9 years ago by novoselt

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!

comment:7 follow-up: Changed 9 years ago by novoselt

  • Status changed from needs_review to needs_work

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 ;-)

comment:8 in reply to: ↑ 7 Changed 9 years ago by hivert

  • Status changed from needs_work to needs_review

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.

comment:9 follow-up: Changed 9 years ago by novoselt

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

comment:10 in reply to: ↑ 9 Changed 9 years ago by hivert

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

comment:11 Changed 9 years ago by novoselt

Successful built with 552 warnings!

comment:12 follow-up: Changed 9 years ago by 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.)

comment:13 in reply to: ↑ 12 Changed 9 years ago by hivert

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.

comment:14 follow-up: Changed 9 years ago by novoselt

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

comment:15 in reply to: ↑ 14 Changed 9 years ago by hivert

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.

comment:16 follow-up: Changed 9 years ago by 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.

comment:17 in reply to: ↑ 16 Changed 9 years ago by hivert

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

comment:18 Changed 9 years ago by novoselt

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).

comment:19 follow-up: Changed 9 years ago by novoselt

I have discovered that

:class:`tuple`

in the documentation translates now into a black link

__builtin__.tuple

in the output. I think it would be better to display links exactly as they were written originally and "expand" only the actual link, if it is working.

comment:20 in reply to: ↑ 19 ; follow-up: Changed 9 years ago by hivert

Replying to novoselt:

I have discovered that

:class:`tuple`

in the documentation translates now into a black link

__builtin__.tuple

in the output. I think it would be better to display links exactly as they were written originally and "expand" only the actual link, if it is working.

Yes ! That was before I know how to resolve those with intersphinx. See my new patches

comment:21 Changed 9 years ago by hivert

  • Description modified (diff)

comment:22 in reply to: ↑ 20 ; follow-up: Changed 9 years ago by hivert

  • Status changed from needs_review to needs_work

Replying to hivert:

Yes ! That was before I know how to resolve those with intersphinx. See my new patches

There is still a problem: currently when linking to python, I raise a warning before intersphinx tries to find the link in Python's database.

comment:23 in reply to: ↑ 22 Changed 9 years ago by hivert

  • Work issues set to Upgrade to Sphinx 1.0

There is still a problem: currently when linking to python, I raise a warning before intersphinx tries to find the link in Python's database.

Some update: It is not possible to achieve what I want in Sphinx 0.6 without hacking into sphinx. However Sphinx 1.0 should be out very soon (they released 1.0beta2 last week). At raising warning is builtin in sphinx 1.0 with the correct configuration option. So I'll probably wait until sphinx 1.0 is out to finish this one, unless someone insist to have it very soon.

comment:24 Changed 9 years ago by novoselt

My opinion is that upgrading Sphinx is the way to go, it may have other benefits (and hopefully no drawbacks). I really want this functionality but I am quite happy with the availability of these patches here, thank you for writing them, Florent!

Andrey

comment:25 Changed 9 years ago by hivert

For the record, here is another bad side effect of this patch:

sage: PolynomialRing?

Type:		function
Base Class:	<type 'function'>
String Form:	<function PolynomialRing at 0x16a2758>
Namespace:	Interactive
File:		/usr/devel/sage/sage/rings/polynomial/polynomial_ring_constructor.py
Definition:	PolynomialRing(base_ring, arg1=None, arg2=None, sparse=False, order='degrevlex', names=None, name=None, implementation=None)
Docstring:
    <no docstring>

comment:26 Changed 9 years ago by novoselt

Is it possible to display line numbers in warnings about bad links?

comment:27 Changed 9 years ago by novoselt

I guess it was expected: these patches do not work anymore in Sage-4.6.1.alpha3 due to Sphinx upgrade. On a fresh installation I got

novoselt@sage:/scratch/novoselt/sage/devel/sage-main$ hg qapplied
novoselt@sage:/scratch/novoselt/sage/devel/sage-main$ hg qpush
applying trac_9128-sphinx_links_all-fh.patch
now at: trac_9128-sphinx_links_all-fh.patch
novoselt@sage:/scratch/novoselt/sage/devel/sage-main$ hg qpush
applying trac_9128-intersphinx_python_database-fh.patch
now at: trac_9128-intersphinx_python_database-fh.patch
novoselt@sage:/scratch/novoselt/sage/devel/sage-main$ ../../sage -b
...
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 v1.0.4
loading pickled environment... done
loading intersphinx inventory from /mnt/usb1/scratch/novoselt/sage/devel/sage/doc/common/python-inv.txt...
building [html]: targets for 1 source files that are out of date
updating environment: [extensions changed] 882 added, 0 changed, 0 removed
reading sources... [100%] tensor                                                                                                                              
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 446, in find_sage_dangling_links
    modname = nodeattr['modname']
KeyError: 'modname'
The full traceback has been saved in /tmp/sphinx-err-5Yrzg5.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
Either send bugs to the mailing list at <http://groups.google.com/group/sphinx-dev/>,
or report them in the tracker at <http://bitbucket.org/birkenfeld/sphinx/issues/>. Thanks!
Build finished.  The built documents can be found in /mnt/usb1/scratch/novoselt/sage/devel/sage/doc/output/html/en/reference

comment:28 Changed 8 years ago by hivert

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Work issues Upgrade to Sphinx 1.0 deleted

I upgraded my patch to the new sphinx. I'm not sure the patch is completely finished but I'd be very interesting to have feedback.

comment:29 Changed 8 years ago by hivert

  • Description modified (diff)
Note: See TracTickets for help on using tickets.