sagemath/sage

Refactor src/sage/docs

kwankyu opened this issue · 40 comments

We deprecate the package sage.docs which contains two modules conf and instancedoc.

The module conf is not one of normal sage modules, but is the Sphinx configuration for documentation building. Hence we move the module into sage_docbuild.

The module instancedoc provides a decorator to add docstrings to instance objects. This has nothing to do with building documentation. Hence we move it to sage.misc.

This ticket also refactors the code in sage_docbuild.__init__. The "builders" are moved to a new module sage_docbuild.builders and the "main" function is moved into sage_docbuild.__main__.

Finally we add some documentation about Sage's documentation system.


To minimize confusion, this ticket should be merged with sage 9.7.beta as early as possible (before any other tickets touching conf.py)

CC: @antonio-rojas

Component: documentation

Author: Kwankyu Lee

Branch: d8a645f

Reviewer: Matthias Koeppe

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

comment:1

Indeed. Why does this package even exist?

Author: Kwankyu Lee

Description changed:

--- 
+++ 
@@ -1 +1,7 @@
+We deprecate the package `sage.docs` which contains two modules `conf` and `instancedoc`.
 
+The module `conf` is not one of normal sage modules, but is the Sphinx configuration for documentation building. Hence we move the modules into `sage_docbuild`. 
+
+The module `instancedoc` provides a decorator to add docstrings to instance objects. This has nothing to do with building documentation. Hence we move it to `sage.misc`.
+
+This ticket also refactors the code in `sage_docbuild.__init__`. The "builders" are moved to a new module `sage_docbuild.builders` and the "main" function is moved into `sage_docbuild.__main__`.
comment:2

New commits:

8511370Refactor sage/docs

Commit: 8511370

Description changed:

--- 
+++ 
@@ -1,7 +1,9 @@
 We deprecate the package `sage.docs` which contains two modules `conf` and `instancedoc`.
 
-The module `conf` is not one of normal sage modules, but is the Sphinx configuration for documentation building. Hence we move the modules into `sage_docbuild`. 
+The module `conf` is not one of normal sage modules, but is the Sphinx configuration for documentation building. Hence we move the module into `sage_docbuild`. 
 
 The module `instancedoc` provides a decorator to add docstrings to instance objects. This has nothing to do with building documentation. Hence we move it to `sage.misc`.
 
 This ticket also refactors the code in `sage_docbuild.__init__`. The "builders" are moved to a new module `sage_docbuild.builders` and the "main" function is moved into `sage_docbuild.__main__`.
+
+Finally we add some documentation about Sage's documentation system.
comment:4

Replying to @mkoeppe:

Indeed. Why does this package even exist?

Removing it is the goal of this ticket. To minimize confusion, this ticket should be the first one to be merged with sage 9.7.beta.

Description changed:

--- 
+++ 
@@ -7,3 +7,7 @@
 This ticket also refactors the code in `sage_docbuild.__init__`. The "builders" are moved to a new module `sage_docbuild.builders` and the "main" function is moved into `sage_docbuild.__main__`.
 
 Finally we add some documentation about Sage's documentation system.
+
+---
+
+**To minimize confusion, this ticket should be the first one to be merged with sage 9.7.beta.**
comment:5

Replying to @mkoeppe:

Indeed. Why does this package even exist?

I don't know...

Changed commit from 8511370 to 4b0fae4

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

4b0fae4Some obvious edits

Description changed:

--- 
+++ 
@@ -10,4 +10,4 @@
 
 ---
 
-**To minimize confusion, this ticket should be the first one to be merged with sage 9.7.beta.**
+**To minimize confusion, this ticket should be merged with sage 9.7.beta as early as possible (before any other tickets touching `conf.py`**

Description changed:

--- 
+++ 
@@ -10,4 +10,4 @@
 
 ---
 
-**To minimize confusion, this ticket should be merged with sage 9.7.beta as early as possible (before any other tickets touching `conf.py`**
+**To minimize confusion, this ticket should be merged with sage 9.7.beta as early as possible (before any other tickets touching `conf.py`)**

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

6517de2More edits

Changed commit from 4b0fae4 to 6517de2

comment:10

The failing Build&Test has nothing to do with this ticket.

comment:11

LGTM.

Reviewer: Matthias Koeppe

comment:12

The new module needs to be added to pkgs/sagemath-objects/MANIFEST.in

Changed commit from 6517de2 to d8a645f

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

d8a645fAdded instancedoc.py to sagemath-objects/MANIFEST.in
comment:14

Would this work?

comment:15

Yes, tested with #28925. Thanks!

Changed branch from u/klee/33763 to d8a645f

comment:17

After this ticket, the git status is not clean:

% git status
On branch develop
Your branch is up to date with 'trac/develop'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	src/doc/en/reference/documentation/sage_docbuild/

nothing added to commit but untracked files present (use "git add" to track)

Also, conf.py produces an html file in the documentation with title="MISSING TITLE".

Changed commit from d8a645f to none

comment:18

Replying to @jhpalmieri:

After this ticket, the git status is not clean:

% git status
On branch develop
Your branch is up to date with 'trac/develop'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	src/doc/en/reference/documentation/sage_docbuild/

nothing added to commit but untracked files present (use "git add" to track)

Also, conf.py produces an html file in the documentation with title="MISSING TITLE".

Will be fixed in #33922.

comment:19

This adds a runtime dependency to sage_docbuild in sagelib (when displaying a command's help). Is this intentional?

comment:20

Replying to @antonio-rojas:

This adds a runtime dependency to sage_docbuild in sagelib (when displaying a command's help).

Sorry. Would you elaborate? What does it mean "when displaying a command's help"?

comment:21

Replying to @kwankyu:

Sorry. Would you elaborate? What does it mean "when displaying a command's help"?

If sage_docbuild is not installed:

sage: integrate?
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
File /usr/lib/python3.10/site-packages/sphinx/config.py:332, in eval_config_file(filename, tags)
    331         code = compile(f.read(), filename.encode(fs_encoding), 'exec')
--> 332         exec(code, namespace)
    333 except SyntaxError as err:

File /tmp/tmpovois2g5/en/introspect/conf.py:2, in <module>
----> 2 from sage_docbuild.conf import *
      3 extensions = ['sphinx.ext.autodoc', 'sphinx.ext.mathjax', 'sphinx.ext.todo', 'sphinx.ext.extlinks']

ModuleNotFoundError: No module named 'sage_docbuild'

The above exception was the direct cause of the following exception:

ConfigError                               Traceback (most recent call last)
Input In [1], in <cell line: 1>()
----> 1 get_ipython().run_line_magic('pinfo', 'integrate')

File /usr/lib/python3.10/site-packages/IPython/core/interactiveshell.py:2304, in InteractiveShell.run_line_magic(self, magic_name, line, _stack_depth)
   2302     kwargs['local_ns'] = self.get_local_scope(stack_depth)
   2303 with self.builtin_trap:
-> 2304     result = fn(*args, **kwargs)
   2305 return result

File /usr/lib/python3.10/site-packages/IPython/core/magics/namespace.py:58, in NamespaceMagics.pinfo(self, parameter_s, namespaces)
     56     self.psearch(oname)
     57 else:
---> 58     self.shell._inspect('pinfo', oname, detail_level=detail_level,
     59                         namespaces=namespaces)

File /usr/lib/python3.10/site-packages/IPython/core/interactiveshell.py:1684, in InteractiveShell._inspect(self, meth, oname, namespaces, **kw)
   1682     pmethod(info.obj, oname, formatter)
   1683 elif meth == 'pinfo':
-> 1684     pmethod(
   1685         info.obj,
   1686         oname,
   1687         formatter,
   1688         info,
   1689         enable_html_pager=self.enable_html_pager,
   1690         **kw,
   1691     )
   1692 else:
   1693     pmethod(info.obj, oname)

File /usr/lib/python3.10/site-packages/IPython/core/oinspect.py:698, in Inspector.pinfo(self, obj, oname, formatter, info, detail_level, enable_html_pager, omit_sections)
    665 def pinfo(
    666     self,
    667     obj,
   (...)
    673     omit_sections=(),
    674 ):
    675     """Show detailed information about an object.
    676 
    677     Optional arguments:
   (...)
    696     - omit_sections: set of section keys and titles to omit
    697     """
--> 698     info = self._get_info(
    699         obj, oname, formatter, info, detail_level, omit_sections=omit_sections
    700     )
    701     if not enable_html_pager:
    702         del info['text/html']

File /usr/lib/python3.10/site-packages/IPython/core/oinspect.py:591, in Inspector._get_info(self, obj, oname, formatter, info, detail_level, omit_sections)
    571 def _get_info(
    572     self, obj, oname="", formatter=None, info=None, detail_level=0, omit_sections=()
    573 ):
    574     """Retrieve an info dict and format it.
    575 
    576     Parameters
   (...)
    588         Titles or keys to omit from output (can be set, tuple, etc., anything supporting `in`)
    589     """
--> 591     info = self.info(obj, oname=oname, info=info, detail_level=detail_level)
    593     _mime = {
    594         'text/plain': [],
    595         'text/html': '',
    596     }
    598     def append_field(bundle, title:str, key:str, formatter=None):

File /usr/lib/python3.10/site-packages/IPython/core/oinspect.py:762, in Inspector.info(self, obj, oname, info, detail_level)
    760             ds += "\nDocstring:\n" + obj.__doc__
    761 else:
--> 762     ds = getdoc(obj)
    763     if ds is None:
    764         ds = '<no docstring>'

File /usr/lib/python3.10/site-packages/sage/misc/lazy_import.pyx:391, in sage.misc.lazy_import.LazyImport.__call__ (build/cythonized/sage/misc/lazy_import.c:4273)()
    389         True
    390     """
--> 391     return self.get_object()(*args, **kwds)
    392 
    393 def __repr__(self):

File /usr/lib/python3.10/site-packages/sage/misc/sageinspect.py:2050, in sage_getdoc(obj, obj_name, embedded)
   2048     return ''
   2049 r = sage_getdoc_original(obj)
-> 2050 s = sage.misc.sagedoc.format(r, embedded=embedded)
   2051 f = sage_getfile(obj)
   2052 if f and os.path.exists(f):

File /usr/lib/python3.10/site-packages/sage/misc/sagedoc.py:752, in format(s, embedded)
    750         s = process_mathtt(s)
    751     s = process_extlinks(s, embedded=embedded)
--> 752     s = detex(s, embedded=embedded)
    753 return s

File /usr/lib/python3.10/site-packages/sage/misc/sagedoc.py:249, in detex(s, embedded)
    247     s = s.replace('``', '"').replace('`', '') + '\n'
    248 else:
--> 249     s = sphinxify(s, format='text')
    250 # Do math substitutions. The strings to be replaced should be
    251 # TeX commands like "\\blah". Do a regular expression
    252 # replacement to replace "\\blah" but not "\\blahxyz", etc.:
    253 # test to make sure the next character is not a letter.
    254 for a,b in math_substitutes:

File /usr/lib/python3.10/site-packages/sage/misc/sphinxify.py:122, in sphinxify(docstring, format)
    119 old_sys_path = list(sys.path)  # Sphinx modifies sys.path
    120 # Sphinx constructor: Sphinx(srcdir, confdir, outdir, doctreedir,
    121 # buildername, confoverrides, status, warning, freshenv).
--> 122 sphinx_app = Sphinx(srcdir, confdir, outdir, doctreedir, format,
    123                     confoverrides, None, None, True)
    124 sphinx_app.build(None, [rst_name])
    125 sys.path = old_sys_path

File /usr/lib/python3.10/site-packages/sphinx/application.py:202, in Sphinx.__init__(self, srcdir, confdir, outdir, doctreedir, buildername, confoverrides, status, warning, freshenv, warningiserror, tags, verbosity, parallel, keep_going)
    200 else:
    201     self.confdir = abspath(confdir)
--> 202     self.config = Config.read(self.confdir, confoverrides or {}, self.tags)
    204 # initialize some limited config variables before initialize i18n and loading
    205 # extensions
    206 self.config.pre_init_values()

File /usr/lib/python3.10/site-packages/sphinx/config.py:165, in Config.read(cls, confdir, overrides, tags)
    162 if not path.isfile(filename):
    163     raise ConfigError(__("config directory doesn't contain a conf.py file (%s)") %
    164                       confdir)
--> 165 namespace = eval_config_file(filename, tags)
    166 return cls(namespace, overrides or {})

File /usr/lib/python3.10/site-packages/sphinx/config.py:345, in eval_config_file(filename, tags)
    343     except Exception as exc:
    344         msg = __("There is a programmable error in your configuration file:\n\n%s")
--> 345         raise ConfigError(msg % traceback.format_exc()) from exc
    347 return namespace

ConfigError: There is a programmable error in your configuration file:

Traceback (most recent call last):
  File "/usr/lib/python3.10/site-packages/sphinx/config.py", line 332, in eval_config_file
    exec(code, namespace)
  File "/tmp/tmpovois2g5/en/introspect/conf.py", line 2, in <module>
    from sage_docbuild.conf import *
ModuleNotFoundError: No module named 'sage_docbuild'
comment:23

Where is this from?

File /tmp/tmpovois2g5/en/introspect/conf.py:2, in <module>
----> 2 from sage_docbuild.conf import *

I could not find .../introspect/conf.py in the sage source tree.

comment:24

It's from these lines from misc/sphinxify.py:

    confdir = os.path.join(srcdir, 'en' , 'introspect')
    os.makedirs(confdir)
    with open(os.path.join(confdir, 'conf.py'), 'w') as filed:
        filed.write(r"""
          ...
comment:25

Replying to @antonio-rojas:

This adds a runtime dependency to sage_docbuild in sagelib (when displaying a command's help). Is this intentional?

That was not intentional. I was not aware that putting conf.py in the directory sage_docbuild introduces the runtime dependency.

I think we may move conf.py to misc/conf.py. Before I open a ticket for that, I want Matthias to confirm that the runtime independence of sagelib from sage_docbuild should not be broken.

Matthias?

comment:26

Sorry, I didn't catch this when reviewing the ticket. sage_docbuild definitely must not be a runtime dependency of sagelib.

comment:27

I don't think all of conf.py is appropriate to go into misc.
Let's split it.
For example, I don't think any of the mathjax configuration is needed for sphinxify.

comment:28

See also: #33682 Replace sage.misc.sphinxify with docrepr

comment:29

Replying to @mkoeppe:

For example, I don't think any of the mathjax configuration is needed for sphinxify.

I'm probably wrong about mathjax here. But some other settings in conf.py are not appropriate in the context of sphinxify:

  • templates_path (which makes a reference to SAGE_DOC_SRC - which is not available at sagelib runtime)
  • jupyter_sphinx_thebelab_config
  • latex_elements
  • add_page_context
  • ...
comment:30

Replying to @mkoeppe:

Replying to @mkoeppe:

For example, I don't think any of the mathjax configuration is needed for sphinxify.

I'm probably wrong about mathjax here.

Well, our only uses of sphinxify with format='html' in the Sage library appears in a function browse_sage_doc (which I just learned about), which does not seem to use mathjax.

comment:31

Replying to @kwankyu:

Before I open a ticket for that, I want Matthias to confirm that the runtime independence of sagelib from sage_docbuild should not be broken.

I've opened #33936 (Remove runtime dependency on sage_docbuild introduced in #33763)