♻️ REFACTOR: Delegate ToC logic to sphinx-external-toc #1293

chrisjsewell · 2021-04-08T22:02:06Z

Ok, say goodbye to half the jupyter-book code base 😆 !

As discussed in executablebooks/meta#292 and then executablebooks/sphinx-external-toc#1, all the external ToC logic has been re-written into a proper sphinx extension:

The format of the ToC file has been changed slightly, to streamline it (see the issue above).
To aide users, the code in jb build will check the format is as expected before calling sphinx,
and point users to the new jupyter-book toc migrate command, e.g.

RuntimeError: 
===============================================================================

The Table of Contents file is malformed: 'root' key not found: '/'
You may need to migrate from the old format, using:

      jupyter-book toc migrate /Users/chrisjsewell/Documents/GitHub/jupyter-book/docs/_toc.yml

===============================================================================

This now also includes glob (in addition to file and url), to specify multiple files with unix-style wildcards.
All toctree options are available: caption, hidden, maxdepth, numbered, titlesonly, reversed.
titlesonly is now set to False by default, rather than True, as is the case for standard sphinx.
All parsing/writing goes via a SiteMap API object, which is stored in sphinx at app.env.external_site_map
The new create_site_from_toc functionality allows a template project to be generated directly from a toc.yml, and is also used for all the testing which is extensive (i.e. you can just create all the files/folders dynamically, before running sphinx)
The toctree are now added directly to the docutils/sphinx AST, rather than appending to the source text. This means that its is source format agnostic, i.e. will work for rst, md, ipynb, py, ...
The tableofcontents directive is also handled at the same time as the toctree insertion, rather than later in a post-transform, which avoids having to use any builder specific logic

The jupyter-book CLI now accepts plugin commands, via the jb.cmdline entry-point. This means that the CLI that I have written in sphinx-external-toc also shows up magically in the jupyter-book CLI 😄 :

$ jupyter-book toc
Usage: jupyter-book toc [OPTIONS] COMMAND [ARGS]...

  Command-line for ``sphinx-external-toc``.

Options:
  --version   Show the version and exit.
  -h, --help  Show this message and exit.

Commands:
  create-site  Create a site from a ToC file.
  create-toc   Create a ToC file from a file structure.
  parse-toc    Parse a ToC file to a site-map YAML.

You'll note that this CLI now has sub-commands, i.e. the existing jb toc -> jb toc create-toc

Removes nested-lookup dependency
Also added a pyproject.toml, and isort to the pre-commit

This is getting close to completion, still to do:

and also obviously it needs a careful review

jupyter_book/sphinx.py

chrisjsewell · 2021-04-08T22:20:20Z

In terms of the ToC format migration, I found some of the current logic/documentation as clear as mud lol 😬.
For example, looking in the test fixtures I found this:

- file: index
  title: Toc
  numbered: true
  chapters:
  - file: content1
    title: Content1
  - file: subfolder/index
    title: Subfolder
    sections:
    - file: subfolder/asubpage
      title: Asubpage
- file: content2
  title: Content2
- file: content3

which I'm not really sure what it means, when there is chapters under the root document, but then also extra file list items at the same level as it.
I believe this migrates to:

defaults:
  numbered: true
root: index
title: Toc
sections:
- file: content1
  title: Content1
- file: subfolder/index
  title: Subfolder
  sections:
  - file: subfolder/asubpage
    title: Asubpage
- file: content2
  title: Content2
- file: content3

is this correct? i.e. it is just equivalent to having a single toctree in the index file:

.. toctree::
   :numbered:

   content1 "Content1"
   subfolder/index "Subfolder"
   content2 "Content2"
   content3

and a single toctree in the subfolder/index file:

.. toctree::
   :numbered:

   subfolder/asubpage "Asubpage"

TomasBeuzen · 2021-04-08T23:29:02Z

Someone will need to update the cookiecutter package with the new ToC format (maybe @TomasBeuzen)

Copy that - I'll take care of that.

AakashGfude · 2021-04-09T03:26:29Z

In terms of the ToC format migration, I found some of the current logic/documentation as clear as mud lol.
For example, looking in the test fixtures I found this ...

is this correct? i.e. it is just equivalent to having a single toctree in the index file:

That looks correct.

Also, the above toctree is the same as? :

defaults:
  numbered: true
root: index
title: Toc
parts:
  - sections:
      - file: content1
        title: Content1
      - file: subfolder/index
        title: Subfolder
        parts:
          - sections:
              - file: subfolder/asubpage
                title: Asubpage
      - file: content2
        title: Content2
      - file: content3

from the example given in the readme https://github.com/executablebooks/sphinx-external-toc#basic-structure .

Also, to be clear, we are not using the single level -file structure now right? like:

defaults:
  numbered: true
root: index
title: Toc
- file: content1
  title: Content1
- file: subfolder/index
  title: Subfolder
  - sections:
      - file: subfolder/asubpage
        title: Asubpage
  - file: content2
    title: Content2
  - file: content3

we will always have sections or parts after the root file?

chrisjsewell · 2021-04-09T05:58:36Z

Also, the above toctree is the same as?

we will always have sections or parts after the root file?

Yes that's correct 👍

- file: intro
- file: doc1
- file: doc2

Will always be:

root: intro
sections:
- file: doc1
- file: doc2

which I think makes it clearer what the final structure will actually be, and allows for the addition of other keys like defaults and meta

- make `test_corrupt_toc` compatible with new code - move `test_toc_numbered` to `test_build`

chrisjsewell · 2021-04-23T22:31:19Z

tests/test_tocdirective/test_toc_latex_parts.tex

-\end{itemize}
-\end{itemize}
-\end{itemize}
+\part{A section}


Here (with jupyterbook-latex==0.2.0) the part headings were missing, in a toc that contains parts

chrisjsewell · 2021-04-23T22:36:57Z

tests/test_tocdirective/test_toc_latex_urllink.tex



-\chapter{A subpage}
+\section{A subpage}


here (with jupyterbook-latex==0.2.0) the header from a sub-toctree was incorrectly assigned as a chapter

but here actually jupyterbook-latex should probably override latex_toplevel_sectioning to section, i.e.:

tocs with format jb-article: set latex_toplevel_sectioning = "section" and don't convert toctree captions to headers

tocs with format jb-book and no parts (i.e. only a single top-level toctree): set latex_toplevel_sectioning = "chapter" and don't convert toctree captions to headers

tocs with format jb-book and parts (i.e. multiple top-level toctrees): set latex_toplevel_sectioning = "part" and do convert toctree captions to headers

chrisjsewell · 2021-04-23T22:42:40Z

@mmcky and @AakashGfude need to update jupyterbook-latex to work with this

I have updated the jupyterbook-latex version here to 0.2.1a1, which was created from the current state of my re-write of that package: executablebooks/sphinx-jupyterbook-latex#55
It fixes the docs build, but also actually fixes two regression tests that are clearly broken with 0.2.0 🤔 (see the comments above)

choldgraf · 2021-04-24T14:15:22Z

@chrisjsewell what are the remaining blockers on this PR? From the comments above it seems like we need to

Make a release of the external toc package
Fix some stuff in Jupyter book-latex
anything else? An

and maybe more specifically, what can I do to help move things along?

chrisjsewell · 2021-04-24T14:37:19Z

what are the remaining blockers on this PR? From the comments above it seems like we need to

list moved to top comment

mmcky · 2021-04-26T01:16:49Z

~~I have done some further testing this morning across two quantecon projects and only came across one issue I have posted on the associated jupyterbook-latex PR~~ This issue is now resolved (thanks @chrisjsewell)

mmcky · 2021-04-27T04:57:33Z

#1293 (comment)

re: setting an option to output the migrated _toc.yml using migrate. It wasn't very difficult to copy and past the output that was given. So I don't mind either way on this. Also happy to make that improvement as a separate PR once this is merged.

My final comment @chrisjsewell is the messages seem to use full-paths such as this in the example above in the PR description

jupyter-book toc migrate /Users/chrisjsewell/Documents/GitHub/jupyter-book/docs/_toc.yml

but I was wondering if relative path may be better as most users are often in the directory jupyter-book (in the example above) and perhaps jupyter-book toc migrate docs/_toc.yml might be more friendly. This again isn't a big issue.

mmcky · 2021-05-03T05:46:46Z

hey @chrisjsewell @choldgraf I have committed 947b6c0 which just adjusts the waitUntil time for pyppeteer to be a little less stringent. This seems to allow pyppeteer to render the pdf in my local environment and the pdfhtml test to pass. The pdf still looks incomplete with missing nodes (i.e. singlehtml issue).

This setting is:

``networkidle2``: when there are no more than 2 network connections
            for at least 500 ms.

this is the resultant pdf:

book.pdf

@chrisjsewell wasn't sure how to resolve the versioning pinning conflict above. Would you mind to take a look?

mmcky · 2021-05-04T00:46:17Z

@chrisjsewell I have fixed this merge conflict in 5046976 by choosing the highest version pinning for each package.

choldgraf · 2021-05-04T03:04:54Z

I've taken our second checklist in the comments and moved it to the top comment, so we have one checklist to follow. That said, I think some of the things in there are not strictly blocking this PR, and are more like follow-ups. @chrisjsewell could you clarify what you need input on to get this PR merged? We have 0.1.0 released for the external TOC extension, and we're gonna make a docs update in a follow-up PR. What else should be done here?

♻️REFACTOR: Delegate ToC logic to sphinx-external-toc

bd28cd4

chrisjsewell requested review from choldgraf, AakashGfude and mmcky April 8, 2021 22:02

chrisjsewell mentioned this pull request Apr 8, 2021

🐛 FIX: Allow skip_text to be a string #1271

Closed

chrisjsewell commented Apr 8, 2021

View reviewed changes

jupyter_book/sphinx.py Outdated Show resolved Hide resolved

chrisjsewell mentioned this pull request Apr 8, 2021

Add filename globbing support to our _toc.yml file #1038

Closed

chrisjsewell linked an issue Apr 8, 2021 that may be closed by this pull request

Add filename globbing support to our _toc.yml file #1038

Closed

This was referenced Apr 8, 2021

Add the toctree just underneath the title instead of at the end of the page #1017

Open

Use custom format file as parent in nested TOC #1064

Closed

chrisjsewell linked an issue Apr 8, 2021 that may be closed by this pull request

Use custom format file as parent in nested TOC #1064

Closed

chrisjsewell mentioned this pull request Apr 8, 2021

Allow other files besides .ipynb, .md and .rst become the index or section head of the book #1175

Closed

chrisjsewell linked an issue Apr 8, 2021 that may be closed by this pull request

Allow other files besides .ipynb, .md and .rst become the index or section head of the book #1175

Closed

chrisjsewell mentioned this pull request Apr 8, 2021

Improve TOC generation test infrastructure #1272

Closed

chrisjsewell linked an issue Apr 8, 2021 that may be closed by this pull request

Improve TOC generation test infrastructure #1272

Closed

This was referenced Apr 8, 2021

CLI toc Generation - Sort Order #1276

Open

Sphinx build fails when toc header contains a colon #742

Closed

This was linked to issues Apr 8, 2021

Sphinx build fails when toc header contains a colon #742

Closed

Wild-cards in _toc.yml #974

Closed

chrisjsewell mentioned this pull request Apr 8, 2021

jb toc does not order pages alphabetically (in my GitHub action only?) #824

Open

chrisjsewell linked an issue Apr 8, 2021 that may be closed by this pull request

Improve error message if a toc.yml has no landing page #1020

Closed

chrisjsewell mentioned this pull request Apr 8, 2021

Improve error message if a toc.yml has no landing page #1020

Closed

TomasBeuzen mentioned this pull request Apr 8, 2021

Update cookiecutter based on new ToC format executablebooks/cookiecutter-jupyter-book#25

Closed

chrisjsewell mentioned this pull request Apr 8, 2021

👌 IMPROVE: Master doc title specified in _toc.yml is visible in left-sidebar executablebooks/sphinx-book-theme#304

Closed

update sphinx-external-toc==0.1.0a3

7bd99e1

rework some tests

eb93259

- make `test_corrupt_toc` compatible with new code - move `test_toc_numbered` to `test_build`

⬆️ UPDATE: jupyterbook-latex==0.2.1a1

e48f20c

chrisjsewell commented Apr 23, 2021

View reviewed changes

choldgraf mentioned this pull request Apr 28, 2021

Subsections get lost when using ReSt as input language #1309

Open

mmcky mentioned this pull request Apr 28, 2021

Documentation build hangs indefinitely for pdfhtml on master #1308

Closed

[TEMP] Apply less stringent waitUnitl condition for pyppeteer rendering

947b6c0

choldgraf mentioned this pull request May 3, 2021

Sphinx External TOC merge into Jupyter Book 2i2c-org/upstream#10

Closed

4 tasks

Merge branch 'master' into sphinx-external-toc

5046976

chrisjsewell added 4 commits May 5, 2021 01:22

update versions

3d83585

fix test

d8a376f

update to 0.11.0a1 for release

abb74c0

jupyterbook-latex~=0.3.0

8595fe8

chrisjsewell merged commit 18d5270 into master May 5, 2021

chrisjsewell deleted the sphinx-external-toc branch May 5, 2021 01:07

This was referenced May 5, 2021

Files not included in TOC tree with .rst files are used as index / header #786

Closed

Jupyter-book version rougier/from-python-to-numpy#95

Open

choldgraf mentioned this pull request May 5, 2021

Updating for new TOC structure #1321

Merged

5 tasks

This was referenced May 11, 2021

Exit with non-zero status if notebooks encounter an unexpected exception. #1320

Open

The Table of Contents file is malformed: toc is not a mapping: <class 'list'> #1329

Closed

This was referenced Aug 9, 2021

Support ReadTheDocs #1091

Closed

✨ NEW: Allow running on ReadTheDocs #1422

Merged

chrisjsewell mentioned this pull request Apr 5, 2022

Dynamic configuration / environment variables / etc with book builds #1673

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

♻️ REFACTOR: Delegate ToC logic to sphinx-external-toc #1293

♻️ REFACTOR: Delegate ToC logic to sphinx-external-toc #1293

chrisjsewell commented Apr 8, 2021 •

edited

Loading

chrisjsewell commented Apr 8, 2021 •

edited

Loading

TomasBeuzen commented Apr 8, 2021 •

edited

Loading

AakashGfude commented Apr 9, 2021 •

edited by chrisjsewell

Loading

chrisjsewell commented Apr 9, 2021 •

edited

Loading

chrisjsewell Apr 23, 2021 •

edited

Loading

chrisjsewell May 5, 2021

chrisjsewell Apr 23, 2021 •

edited

Loading

chrisjsewell Apr 23, 2021

chrisjsewell May 5, 2021

chrisjsewell commented Apr 23, 2021

choldgraf commented Apr 24, 2021

chrisjsewell commented Apr 24, 2021 •

edited by choldgraf

Loading

mmcky commented Apr 26, 2021 •

edited

Loading

mmcky commented Apr 27, 2021

mmcky commented May 3, 2021 •

edited

Loading

mmcky commented May 4, 2021

choldgraf commented May 4, 2021



		\chapter{A subpage}
		\section{A subpage}

♻️ REFACTOR: Delegate ToC logic to sphinx-external-toc #1293

♻️ REFACTOR: Delegate ToC logic to sphinx-external-toc #1293

Conversation

chrisjsewell commented Apr 8, 2021 • edited Loading

chrisjsewell commented Apr 8, 2021 • edited Loading

TomasBeuzen commented Apr 8, 2021 • edited Loading

AakashGfude commented Apr 9, 2021 • edited by chrisjsewell Loading

chrisjsewell commented Apr 9, 2021 • edited Loading

chrisjsewell Apr 23, 2021 • edited Loading

Choose a reason for hiding this comment

chrisjsewell May 5, 2021

Choose a reason for hiding this comment

chrisjsewell Apr 23, 2021 • edited Loading

Choose a reason for hiding this comment

chrisjsewell Apr 23, 2021

Choose a reason for hiding this comment

chrisjsewell May 5, 2021

Choose a reason for hiding this comment

chrisjsewell commented Apr 23, 2021

choldgraf commented Apr 24, 2021

chrisjsewell commented Apr 24, 2021 • edited by choldgraf Loading

mmcky commented Apr 26, 2021 • edited Loading

mmcky commented Apr 27, 2021

mmcky commented May 3, 2021 • edited Loading

mmcky commented May 4, 2021

choldgraf commented May 4, 2021

chrisjsewell commented Apr 8, 2021 •

edited

Loading

chrisjsewell commented Apr 8, 2021 •

edited

Loading

TomasBeuzen commented Apr 8, 2021 •

edited

Loading

AakashGfude commented Apr 9, 2021 •

edited by chrisjsewell

Loading

chrisjsewell commented Apr 9, 2021 •

edited

Loading

chrisjsewell Apr 23, 2021 •

edited

Loading

chrisjsewell Apr 23, 2021 •

edited

Loading

chrisjsewell commented Apr 24, 2021 •

edited by choldgraf

Loading

mmcky commented Apr 26, 2021 •

edited

Loading

mmcky commented May 3, 2021 •

edited

Loading