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

add intersphinx mapping for SciPy #29231

Closed
mwageringel opened this issue Feb 21, 2020 · 28 comments · Fixed by #37575
Closed

add intersphinx mapping for SciPy #29231

mwageringel opened this issue Feb 21, 2020 · 28 comments · Fixed by #37575

Comments

@mwageringel
Copy link

This ticket adds an intersphinx mapping for Sphinx to resolve hyperlinks to the Scipy documentation.

With this change, Sphinx fetches the Scipy inventory file objects.inv from https://docs.scipy.org/doc/scipy/reference/ during the docbuild. It links against the latest version of the Scipy documentation to keep it simple.

If the download fails, for example when building Sage without internet access, the docbuild will still succeed, but the Scipy cross-links will not be resolved.

CC: @embray @jhpalmieri

Component: documentation

Author: Markus Wageringel

Branch/Commit: u/gh-mwageringel/29231 @ d19232d

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

@mwageringel
Copy link
Author

Commit: 3bd5293

@mwageringel
Copy link
Author

comment:1

This merges cleanly with #17405.

If this gets accepted, the same could be applied to numpy.


New commits:

3bd529329231: add intersphinx mapping for scipy

@mwageringel
Copy link
Author

Author: Markus Wageringel

@mwageringel
Copy link
Author

Branch: u/gh-mwageringel/29231

@mwageringel

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:3

Sage should in principle build without an internet connection, so I am not in favor of this approach. (“If this fails for some reason, the docbuild will also fail.”)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 21, 2020

Changed commit from 3bd5293 to 2bd8a8a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 21, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

2bd8a8a29231: fallback for scipy inventory file when not connected to internet

@mwageringel
Copy link
Author

comment:5

You are right, that is a problem. It is possible to set a local inventory file as a fallback*. For Python, this was done by adding src/doc/common/python3.inv to the repository. However, I do not really like the idea of adding another binary file to the repository that would need to be updated manually.

Instead, I have configured intersphinx to use the python3.inv as fallback for Scipy. This means that cross-links for Scipy are not resolved when the fallback is used, as they are not present in the Python inventory file, but the documentation build still succeeds.

With these changes applied, I have successfully built the documentation without internet access.


*For reference, see Multiple target for the inventory.

@mwageringel

This comment has been minimized.

@mwageringel mwageringel changed the title add intersphinx mapping for Scipy add intersphinx mapping for SciPy Feb 21, 2020
@videlec
Copy link
Contributor

videlec commented Feb 22, 2020

comment:6

Replying to @jhpalmieri:

Sage should in principle build without an internet connection, so I am not in favor of this approach. (“If this fails for some reason, the docbuild will also fail.”)

+1.

Though, it would be great that something like :obj:`scipy:scipy.integrate.ode` in Sage documentation resolves correctly (either pointing to the local documentation or online). pplpy intersphinx relies on the fact that its documentation is built and installed. This is implemented in build/pkgs/pplpy/spkg-postinst

$ cat build/pkgs/pplpy/spkg-postinst 
if [[ "$SAGE_SPKG_INSTALL_DOCS" != no ]] ; then
    # Compile pplpy's documentation
    cd src
    cd docs
    $MAKE html

    # install pplpy's documentation
    PPLPY_DOCS=$SAGE_SHARE/doc/pplpy
    mkdir -p $PPLPY_DOCS
    cp -r build/html/*  $PPLPY_DOCS
fi

Though, if we start allowing for pplpy to come from the system the intersphinx will be broken and that also needs a solution.

@mwageringel
Copy link
Author

comment:7

Replying to @videlec:

Though, if we start allowing for pplpy to come from the system the intersphinx will be broken and that also needs a solution.

I would suggest to change the pplpy intersphinx mapping to point to the online version of the pplpy documentation instead of the local version. The local inventory file can be used as fallback (and the dummy file as secondary fallback). This ensures that cross-links (to the online version) can still be resolved correctly if either an internet connection or the local pplpy documentation exists during the docbuild.

Admittedly, it might be better if the cross-links resolved to the local documentation in the case when the local objects.inv is used, and to the online documentation if the online version is used, but there does not seem to be an easy way to configure this, as far as I understand.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 24, 2020

Changed commit from 2bd8a8a to ad16967

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 24, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

ad1696729231: use online pplpy docs for intersphinx with local fallback

@mwageringel
Copy link
Author

comment:9

I have just made the change for pplpy as well, so Intersphinx gets the online version of the objects.inv file and uses the local version as fallback. I have successfully built the documentation without internet access. There do not seem to be any pplpy cross-links, so far. The dochtml.log shows that Intersphinx correctly finds the fallback, though.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 25, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

d19232d29231: fix pyflakes warnings

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 25, 2020

Changed commit from ad16967 to d19232d

@videlec
Copy link
Contributor

videlec commented Feb 26, 2020

comment:11

This proposal is not acceptable. Building Sage should not trigger any internet connection beyond downloading packages (that can be done prior to the build with sage -d).

One option would actually be to package the various inventories. But that look tedious to maintain.

Do you know what other projects do?

@mwageringel
Copy link
Author

comment:12

Other projects like numpy, scipy and matplotlib all use the online versions of inventory files. None of them even provides a fallback for the case that the download fails. This also means we cannot follow the pplpy approach of compiling the documentation for those packages ourselves, as it would connect to the internet.

I am not sure why triggering an internet connection is a problem, as long as download failures are handled gracefully. In the worst case, one ends up without cross-references, which is exactly what we have now.

I would like to avoid extra maintenance burdens that would be caused by packaging the inventory files. How about making the download of inventory files optional, similar to the --no-plot docbuild option? It would be nice then if this was used for the documentation on our website, at least, which is probably where most people read the Sage documentation.

@videlec
Copy link
Contributor

videlec commented Feb 26, 2020

comment:13

Replying to @mwageringel:

Other projects like numpy, scipy and matplotlib all use the online versions of inventory files. None of them even provides a fallback for the case that the download fails. This also means we cannot follow the pplpy approach of compiling the documentation for those packages ourselves, as it would connect to the internet.

I am not sure why triggering an internet connection is a problem, as long as download failures are handled gracefully. In the worst case, one ends up without cross-references, which is exactly what we have now.

Because a user do not expect make build to trigger an internet connection. I do not quite agree with the term "handled gracefully" since you end up with a different documentation.

I would like to avoid extra maintenance burdens that would be caused by packaging the inventory files.

+1. It makes no sense to have these files as packages. It will be outdated very soon.

How about making the download of inventory files optional, similar to the --no-plot docbuild option? It would be nice then if this was used for the documentation on our website, at least, which is probably where most people read the Sage documentation.

Could you make a detailed proposal on sage-devel? This kind of changes in the build system have to go through a common agreement by devs.

@mwageringel
Copy link
Author

comment:14

Here is the thread on sage-devel. There is also a very similar ticket #27164.

@jhpalmieri
Copy link
Member

comment:15

There is also #27495, which is somewhat related.

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 Apr 15, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Sep 5, 2020
@embray
Copy link
Contributor

embray commented Feb 26, 2021

comment:18

What's the status of this? There has been discussion of this recently again, and it would be good to do at least something about it, as I need this for #31404 as well.

I think currently this looks good--one thing I might change is for the dummy fallback see if the docs are installed locally first (e.g. if pplpy was installed with SAGE_SPKG_INSTALL_DOCS, then we can find its docs in SAGE_SHARE/doc/pplpy).

I think this is better than nothing for now.

@mwageringel
Copy link
Author

comment:19

I think the main concern voiced on the mailing list discussion linked in comment:14 is that intersphinx eagerly tries to connect to the internet, which may be undesirable for some people in some situations. To avoid this, Sage could by default ask for permission to connect to the internet to download missing files, but that would be quite a complex change to the build system.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 5, 2021

comment:20

We should just make this optional, enabled by another SAGE_DOC_.... option.
When building the version for the website (see also #31415), we would enable the option.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 24, 2021

comment:21

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Mar 24, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 19, 2021

comment:22

Setting a new milestone for this ticket based on a cursory review.

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 18, 2021

comment:23

Stalled in needs_review or needs_info; likely won't make it into Sage 9.5.

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 7, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Aug 31, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
vbraun pushed a commit to vbraun/sage that referenced this issue Mar 30, 2024
…flint, fpylll, gmpy2, ipywidgets, matplotlib, mpmath, networkx, numpy, rpy2, scipy, sympy

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

[Intersphinx](https://www.sphinx-
doc.org/en/master/usage/extensions/intersphinx.html) allows us to refer
to functions and classes in other projects using the standard Sphinx
roles `:func:`,  `:class:`, ...

Examples in the documentation preview:
- [References to FLINT functions in
sage.libs.flint.arith_sage](https://deploy-preview-37575--sagemath.netli
fy.app/html/en/reference/libs/sage/libs/flint/arith_sage#sage.libs.flint
.arith_sage.bell_number)
- [References to SciPy functions in
sage.manifolds.differentiable.integrated_curve](https://deploy-
preview-37575--sagemath.netlify.app/html/en/reference/manifolds/sage/man
ifolds/differentiable/integrated_curve#sage.manifolds.differentiable.int
egrated_curve.IntegratedCurve.solve)
- [References to NetworkX functions in
sage.graphs.generic_graph](https://deploy-preview-37575--sagemath.netlif
y.app/html/en/reference/graphs/sage/graphs/generic_graph#sage.graphs.gen
eric_graph.GenericGraph.export_to_file)
- [Examples in the Developer Guide](https://deploy-preview-37575--
sagemath.netlify.app/html/en/developer/sage_manuals#hyperlinks)

Based on a rebased version of sagemath#29231 by @mwageringel. In addition to the
`scipy` intersphinx done in sagemath#29231, here we add a number of relevant
Python libraries, as well as `flint`, whose docs are built with Sphinx
as well.

To address concerns in the discussion there about the docbuild
contacting remote servers for the inventory files, here we instead
vendor the inventory files:
```
$ ls -l src/doc/common/_vendor
-rw-r--r--  1 mkoeppe  staff    1942 Mar  9 21:20 cvxopt.inv
-rw-r--r--  1 mkoeppe  staff   13251 Mar  9 21:20 cvxpy.inv
-rw-r--r--  1 mkoeppe  staff   10728 Mar  9 21:20 cypari2.inv
-rw-r--r--  1 mkoeppe  staff     775 Mar  9 21:20 cysignals.inv
-rw-r--r--  1 mkoeppe  staff  246266 Mar  9 21:20 flint.inv
-rw-r--r--  1 mkoeppe  staff    1603 Mar  9 21:20 fpylll.inv
-rw-r--r--  1 mkoeppe  staff    2891 Mar  9 21:20 gmpy2.inv
-rw-r--r--  1 mkoeppe  staff   10934 Mar  9 21:20 ipywidgets.inv
-rw-r--r--  1 mkoeppe  staff  105887 Mar  9 21:20 matplotlib.inv
-rw-r--r--  1 mkoeppe  staff    3115 Mar  9 21:20 mpmath.inv
-rw-r--r--  1 mkoeppe  staff   51830 Mar  9 21:20 networkx.inv
-rw-r--r--  1 mkoeppe  staff   78006 Mar  9 21:20 numpy.inv
-rw-r--r--  1 mkoeppe  staff    1449 Mar  9 21:20 pplpy.inv
-rw-r--r--  1 mkoeppe  staff  136166 Mar  9 21:20 python.inv
-rw-r--r--  1 mkoeppe  staff    3289 Mar  9 21:20 rpy2.inv
-rw-r--r--  1 mkoeppe  staff  112691 Mar  9 21:20 scipy.inv
-rw-r--r--  1 mkoeppe  staff   55596 Mar  9 21:20 sympy.inv
```
This extends our existing practice with the Python inventory (moved here
from its previous location `src/doc/common/python3.inv`). The new
command `sage -python -m sage_docbuild.vendor` retrieves the latest
versions of these files. (Distribution notes: These files are not
installed, as they are only needed at the build time of the
documentation. After sagemath#36730, they are part of the sdist of sagemath-doc-
html, but not part of the wheel.)

By setting `SAGE_DOC_REMOTE_INVENTORIES=yes`, this can be overridden, as
previously suggested in
sagemath#29231 (comment).
In this case it is first tried to contact the remote servers.

Fixes sagemath#29231 (stalled since 2020).
Fixes sagemath#27164 (stalled since 2019).

**Outside the scope of this PR:**
- adding such Intersphinx links everywhere (that would be a long-term
writing project - https://groups.google.com/g/sage-
devel/c/PfYpuOWt8xQ/m/Emn7pZd5AQAJ)
- linking to documentation of non-Sphinx projects (see sagemath#37598, sagemath#37577)
- any considerations how these .inv files could/should/would be taken
from our installations of these packages, or from system packages that
ship the documentation.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37575
Reported by: Matthias Köppe
Reviewer(s): David Coudert, Matthias Köppe
@mkoeppe mkoeppe added this to the sage-10.4 milestone Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment