-
-
Notifications
You must be signed in to change notification settings - Fork 480
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
Intersphinx for cvxopt, cvxpy, cypari2, cysignals, flint, fpylll, gmpy2, ipywidgets, matplotlib, mpmath, networkx, numpy, rpy2, scipy, sympy #37575
Conversation
fd24f9f
to
b20e5b4
Compare
… not contact remotes; use vendored inventories for scipy, flint
b20e5b4
to
4c0c20c
Compare
@@ -27,8 +27,8 @@ | |||
order equations, return list of points. | |||
|
|||
- :func:`desolve_odeint` - Solve numerically a system of first-order ordinary | |||
differential equations using ``odeint`` from `scipy.integrate module. | |||
<https://docs.scipy.org/doc/scipy/reference/integrate.html#module-scipy.integrate>`_ | |||
differential equations using :func:`~scipy:scipy.integrate.odeint` from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure to understand the meaning of the ~
. Here we have :func:`~scipy:scipy.integrate.odeint`
, but line 1502 you use :func:`scipy:scipy.integrate.odeint`
. Why and why is it needed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ~
gives short links, such as "odeint()" instead of "scipy.integrate.odeint()"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I use it because the text already says from what module it comes.
@@ -594,8 +594,8 @@ cdef class Matrix_double_dense(Matrix_numpy_dense): | |||
|
|||
ALGORITHM: | |||
|
|||
Computation is performed by the ``norm()`` function of | |||
the SciPy/NumPy library. | |||
Computation is performed by the :func:`~scipy:scipy.linalg.norm` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, here we have a ~
but not line 742.
``'graphml'`` | :func:`networkx.readwrite.graphml.write_graphml` | ||
``'multiline_adjlist'`` | :func:`networkx.readwrite.multiline_adjlist.write_multiline_adjlist` | ||
``'pajek'`` | :func:`networkx.readwrite.pajek.write_pajek` | ||
``'yaml'`` | :func:`networkx.readwrite.nx_yaml.write_yaml` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I check on the deployed documentation, this link is not working
https://deploy-preview-37575--sagemath.netlify.app/html/en/reference/graphs/sage/graphs/generic_graph#sage.graphs.generic_graph.GenericGraph.export_to_file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this function simply does not exist any more in current versions of networkx. Is it tested anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is no longer in networkx (https://networkx.org/documentation/stable/reference/readwrite/index.html).
It's not tested in method export_to_file
and according to the code of the method, it is no longer in the list of considered formats. I think it has been removed sometime ago and we have forgotten to remove it from the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I remove it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in a55ee28
Documentation preview for this PR (built with commit a55ee28; changes) is ready! 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have checked several links in the generated documentation and it looks good.
However, I have not checked what may happen without Internet connection for both the construction of the documentation and its consultation on a local installation. Should someone check the before accepting this PR ?
Thanks! I'll try right now, so let me disconnNO CARRIER |
Tested: It builds correctly without access to the Internet, the HTML pages render correctly without access, obviously clicking the links to the external documentation give an error page, but after enabling access to the Internet, the links work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for testing. I think it's an interesting feature.
LGTM.
Thanks for the review! |
…ppl, qepcad, scip, singular, soplex <!-- ^ 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". --> We create a standard way to refer to the online manuals of these packages, which are generated by tools other than Sphinx and therefore cannot be linked to using Intersphinx. This extends what has been already done with Pari. Examples: - [Examples in the Developer Guide](https://deploy-preview-37598-- sagemath.netlify.app/html/en/developer/sage_manuals#hyperlinks) Fixes sagemath#27495. Outside of the scope of this PR: - adding such extlinks everywhere - working with upstream projects to upgrade to better documentation tools - projects that can be linked to using Intersphinx; that's sagemath#37575 - any consideration how to link instead to local copies of those manuals, whether installed from SPKG or system packages. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] 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#37598 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe, Max Horn
Intersphinx allows us to refer to functions and classes in other projects using the standard Sphinx roles
:func:
,:class:
, ...Examples in the documentation preview:
Based on a rebased version of #29231 by @mwageringel. In addition to the
scipy
intersphinx done in #29231, here we add a number of relevant Python libraries, as well asflint
, 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:
This extends our existing practice with the Python inventory (moved here from its previous location
src/doc/common/python3.inv
). The new commandsage -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 #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 #29231 (comment). In this case it is first tried to contact the remote servers.Fixes #29231 (stalled since 2020).
Fixes #27164 (stalled since 2019).
Outside the scope of this PR:
breathe
- Sphinx plugin for project documentation using Doxygen #37577)📝 Checklist
⌛ Dependencies