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

using :doc: tag instead of relative links with ../ to html #36622

Closed
dimpase opened this issue Nov 1, 2023 · 6 comments
Closed

using :doc: tag instead of relative links with ../ to html #36622

dimpase opened this issue Nov 1, 2023 · 6 comments

Comments

@dimpase
Copy link
Member

dimpase commented Nov 1, 2023

One can use :doc: tag here:

--- a/src/sage/rings/polynomial/msolve.py
+++ b/src/sage/rings/polynomial/msolve.py
@@ -8,7 +8,7 @@ based on Gröbner bases.
 This module provide implementations of some operations on polynomial ideals
 based on msolve.
 
-Note that the `optional package msolve <../../../../spkg/msolve.html>`_ must be installed.
+Note that the :doc:`optional package msolve <spkg/msolve>` must be installed.
 
 .. SEEALSO::
 

and it looks much more Sphinxic to me. I propose to do this for many other places where this is present - this would also make these links in pdf docs meaningful.

Originally posted by @dimpase in #36611 (comment)

@dimpase
Copy link
Member Author

dimpase commented Nov 1, 2023

It's actually doesn't quite work, i.e. it needs two runs of make to succeed. On the 1st run one gets an error

[sagemath_doc_html-none] [polynomia] The inventory files are in ../../local/share/doc/sage/inventory/en/reference/polynomial_rings.
[sagemath_doc_html-none] Error building the documentation.
[sagemath_doc_html-none] Traceback (most recent call last):
[sagemath_doc_html-none]   File "<frozen runpy>", line 198, in _run_module_as_main
[sagemath_doc_html-none]   File "<frozen runpy>", line 88, in _run_code
[sagemath_doc_html-none]   File "/mnt/opt/Sage/sage-clang/pkgs/sage-docbuild/sage_docbuild/__main__.py", line 508, in <module>
[sagemath_doc_html-none]     sys.exit(main())
[sagemath_doc_html-none]              ^^^^^^
[sagemath_doc_html-none]   File "/mnt/opt/Sage/sage-clang/pkgs/sage-docbuild/sage_docbuild/__main__.py", line 504, in main
[sagemath_doc_html-none]     builder()
[sagemath_doc_html-none]   File "/mnt/opt/Sage/sage-clang/pkgs/sage-docbuild/sage_docbuild/builders.py", line 818, in _wrapper
[sagemath_doc_html-none]     getattr(DocBuilder, build_type)(self, *args, **kwds)
[sagemath_doc_html-none]   File "/mnt/opt/Sage/sage-clang/pkgs/sage-docbuild/sage_docbuild/builders.py", line 162, in f
[sagemath_doc_html-none]     runsphinx()
[sagemath_doc_html-none]   File "/mnt/opt/Sage/sage-clang/pkgs/sage-docbuild/sage_docbuild/sphinxbuild.py", line 327, in runsphinx
[sagemath_doc_html-none]     sys.stderr.raise_errors()
[sagemath_doc_html-none]   File "/mnt/opt/Sage/sage-clang/pkgs/sage-docbuild/sage_docbuild/sphinxbuild.py", line 263, in raise_errors
[sagemath_doc_html-none]     raise OSError(self._error)
[sagemath_doc_html-none] OSError: /mnt/opt/Sage/sage-clang/src/sage/rings/polynomial/msolve.py:docstring of sage.rings.polynomial.msolve:7: WARNING: unknown document: 'spkg/msolve'
[sagemath_doc_html-none] 

So it's a bit of chicken & egg issue here.

@mkoeppe
Copy link
Member

mkoeppe commented Nov 1, 2023

Can't we just refer to this section by label (:ref:spkg_msolve`)?

@dimpase
Copy link
Member Author

dimpase commented Nov 2, 2023

Can't we just refer to this section by label (:ref:spkg_msolve`)?

yes, this works. We have several ../../ other URLs in docstrings, which are probably not so easy to fix. I'll try adding tags, and corresponding :ref:, in places where rst files are in the git tree.

@dimpase
Copy link
Member Author

dimpase commented Nov 2, 2023

OK, the following works. I'll do a PR based on this, to remove as many ../*.html in docstring URLs as possible.

--- a/src/doc/en/reference/asymptotic/index.rst
+++ b/src/doc/en/reference/asymptotic/index.rst
@@ -1,3 +1,5 @@
+.. _asymptotic:
+
 Asymptotic Expansions
 =====================
 
--- a/src/sage/rings/big_oh.py
+++ b/src/sage/rings/big_oh.py
@@ -3,7 +3,7 @@ Big O for various types (power series, p-adics, etc.)
 
 .. SEEALSO::
 
-    - `asymptotic expansions <../../../asymptotic/index.html>`_
+    - :ref:`asymptotic`
     - `p-adic numbers <../../../padics/index.html>`_
     - `power series <../../../power_series/index.html>`_
     - `polynomials <../../../polynomial_rings/index.html>`_
@@ -90,7 +90,7 @@ def O(*x, **kwds):
         sage: K(11^-12, 15)
         11^-12 + O(11^15)
 
-    We can also work with `asymptotic expansions`_::
+    We can also work with :ref:`asymptotic expansions <asymptotic>`::
 
         sage: A.<n> = AsymptoticRing(growth_group='QQ^n * n^QQ * log(n)^QQ',            # needs sage.symbolic
         ....:                        coefficient_ring=QQ); A
--- a/src/sage/rings/polynomial/msolve.py
+++ b/src/sage/rings/polynomial/msolve.py
@@ -8,7 +8,7 @@ based on Gröbner bases.
 This module provide implementations of some operations on polynomial ideals
 based on msolve.
 
-Note that the `optional package msolve <../../../../spkg/msolve.html>`_ must be installed.
+Note that the :ref:`spkg_msolve` must be installed.
 
 .. SEEALSO::
 

@mkoeppe
Copy link
Member

mkoeppe commented Feb 17, 2024

I've put the changes using :ref:`spkg_msolve` on #37376.

vbraun pushed a commit to vbraun/sage that referenced this issue Feb 19, 2024
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->
From
sagemath#36622 (comment)

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] 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
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

URL: sagemath#37376
Reported by: Matthias Köppe
Reviewer(s): Frédéric Chapoton
@dimpase
Copy link
Member Author

dimpase commented Feb 21, 2024

fixed by #37376

@dimpase dimpase closed this as completed Feb 21, 2024
vbraun pushed a commit to vbraun/sage that referenced this issue Feb 22, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->
From
sagemath#36622 (comment)

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] 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
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#37376
Reported by: Matthias Köppe
Reviewer(s): Frédéric Chapoton
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

2 participants