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

Refactor src/sage/docs #33763

Closed
kwankyu opened this issue Apr 27, 2022 · 40 comments
Closed

Refactor src/sage/docs #33763

kwankyu opened this issue Apr 27, 2022 · 40 comments

Comments

@kwankyu
Copy link
Collaborator

kwankyu commented Apr 27, 2022

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

@kwankyu kwankyu added this to the sage-9.6 milestone Apr 27, 2022
@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 27, 2022

comment:1

Indeed. Why does this package even exist?

@kwankyu
Copy link
Collaborator Author

kwankyu commented Apr 27, 2022

Branch: u/klee/33763

@kwankyu
Copy link
Collaborator Author

kwankyu commented Apr 27, 2022

Author: Kwankyu Lee

@kwankyu

This comment has been minimized.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Apr 27, 2022

comment:2

New commits:

8511370Refactor sage/docs

@kwankyu
Copy link
Collaborator Author

kwankyu commented Apr 27, 2022

Commit: 8511370

@kwankyu kwankyu modified the milestones: sage-9.6, sage-9.7 Apr 27, 2022
@kwankyu

This comment has been minimized.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Apr 27, 2022

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.

@kwankyu

This comment has been minimized.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Apr 27, 2022

comment:5

Replying to @mkoeppe:

Indeed. Why does this package even exist?

I don't know...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 27, 2022

Changed commit from 8511370 to 4b0fae4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 27, 2022

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

4b0fae4Some obvious edits

@kwankyu

This comment has been minimized.

@kwankyu

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 27, 2022

Changed commit from 4b0fae4 to 6517de2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 27, 2022

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

6517de2More edits

@kwankyu
Copy link
Collaborator Author

kwankyu commented Apr 28, 2022

comment:10

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 28, 2022

comment:11

LGTM.

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 28, 2022

Reviewer: Matthias Koeppe

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 28, 2022

comment:12

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 29, 2022

comment:15

Yes, tested with #28925. Thanks!

@vbraun
Copy link
Member

vbraun commented May 22, 2022

Changed branch from u/klee/33763 to d8a645f

@jhpalmieri
Copy link
Member

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

@jhpalmieri
Copy link
Member

Changed commit from d8a645f to none

@kwankyu
Copy link
Collaborator Author

kwankyu commented May 28, 2022

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.

@antonio-rojas
Copy link
Contributor

comment:19

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

@kwankyu
Copy link
Collaborator Author

kwankyu commented May 28, 2022

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"?

@antonio-rojas
Copy link
Contributor

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'

@kwankyu
Copy link
Collaborator Author

kwankyu commented May 28, 2022

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.

@jhpalmieri
Copy link
Member

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

@kwankyu
Copy link
Collaborator Author

kwankyu commented May 29, 2022

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?

@mkoeppe
Copy link
Contributor

mkoeppe commented May 29, 2022

comment:26

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

@mkoeppe
Copy link
Contributor

mkoeppe commented May 29, 2022

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.

@mkoeppe
Copy link
Contributor

mkoeppe commented May 29, 2022

comment:28

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

@mkoeppe
Copy link
Contributor

mkoeppe commented May 29, 2022

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

@mkoeppe
Copy link
Contributor

mkoeppe commented May 29, 2022

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.

@mkoeppe
Copy link
Contributor

mkoeppe commented May 29, 2022

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)

mkoeppe added a commit to mkoeppe/sage that referenced this issue Sep 20, 2023
vbraun pushed a commit to vbraun/sage that referenced this issue Sep 23, 2023
    
<!-- ^^^^^
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 -->

<!-- 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#36306
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
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

5 participants