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

More speedups to section TOC rendering #1642

Merged
merged 4 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/pydata_sphinx_theme/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,4 +287,4 @@ def setup(app: Sphinx) -> Dict[str, str]:
# Include component templates
app.config.templates_path.append(str(theme_path / "components"))

return {"parallel_read_safe": True, "parallel_write_safe": True}
return {"parallel_read_safe": True, "parallel_write_safe": False}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So for scipy build time is improved even if it's not written in parallel anymore ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. on this branch serial scipy build is around 8 minutes on my machine. And with this change it's still possible to use parallel build (active only for the read phase) and still shave a few minutes off the build time, without having the sidebar navs come out wrong.

4 changes: 2 additions & 2 deletions src/pydata_sphinx_theme/theme/pydata_sphinx_theme/layout.html
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@
<div class="bd-container">
<div class="bd-container__inner bd-page-width">
{# Primary sidebar #}
{# If we have no sidebar TOC, pop the TOC component from the sidebar list #}
{% if missing_sidebar_toctree(includehidden=theme_sidebar_includehidden) %}
{# If we have no sidebar TOC, pop the TOC component from the sidebars list #}
{% if suppress_sidebar_toctree(includehidden=theme_sidebar_includehidden | tobool) %}
{% set sidebars = sidebars | reject("in", "sidebar-nav-bs.html") | list %}
{% endif %}
<div class="bd-sidebar-primary bd-sidebar{% if not sidebars %} hide-on-wide{% endif %}">
Expand Down
155 changes: 94 additions & 61 deletions src/pydata_sphinx_theme/toctree.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,60 +31,58 @@
)


def _get_ancestor_section(app: Sphinx, pagename: str, startdepth: int) -> str:
"""Get the TocTree node `startdepth` levels below the root that dominates `pagename`."""
def _get_ancestor_pagename(app: Sphinx, pagename: str, startdepth: int) -> str:
"""Get the name of `pagename`'s ancestor that is rooted `startdepth` levels below the global root."""
toctree = TocTree(app.env)
if sphinx.version_info[:2] >= (7, 2):
from sphinx.environment.adapters.toctree import _get_toctree_ancestors

ancestors = [*_get_toctree_ancestors(app.env.toctree_includes, pagename)]
else:
ancestors = toctree.get_toctree_ancestors(pagename)

Check warning on line 42 in src/pydata_sphinx_theme/toctree.py

View check run for this annotation

Codecov / codecov/patch

src/pydata_sphinx_theme/toctree.py#L42

Added line #L42 was not covered by tests
try:
return ancestors[-startdepth] # will be a pagename (string)?
out = ancestors[-startdepth]
except IndexError:
# eg for index.rst, but also special pages such as genindex, py-modindex, search
# those pages don't have a "current" element in the toctree, so we can
# directly return an empty string instead of using the default sphinx
# directly return None instead of using the default sphinx
# toctree.get_toctree_for(pagename, app.builder, collapse, **kwargs)
return None


def get_unrendered_local_toctree(app: Sphinx, pagename: str, startdepth: int, **kwargs):
"""Get the "local" (starting at `startdepth`) TocTree containing `pagename`.

This is similar to `context["toctree"](**kwargs)` in sphinx templating,
but using the startdepth-local instead of global TOC tree.
"""
kwargs.setdefault("collapse", True)
if kwargs.get("maxdepth") == "":
kwargs.pop("maxdepth")
toctree = TocTree(app.env)
indexname = _get_ancestor_section(app=app, pagename=pagename, startdepth=startdepth)
if indexname is None:
return None
return get_local_toctree_for_doc(
toctree, indexname, pagename, app.builder, **kwargs
)
out = None
return out, toctree


def add_toctree_functions(
app: Sphinx, pagename: str, templatename: str, context, doctree
) -> None:
"""Add functions so Jinja templates can add toctree objects."""

def missing_sidebar_toctree(startdepth: int = 1, **kwargs):
def suppress_sidebar_toctree(startdepth: int = 1, **kwargs):
"""Check if there's a sidebar TocTree that needs to be rendered.

Parameters:
startdepth : The level of the TocTree at which to start. 0 includes the
entire TocTree for the site; 1 (default) gets the TocTree for the current
top-level section.

kwargs: passed to the Sphinx `toctree` template function.
kwargs : passed to the Sphinx `toctree` template function.
"""
toctree = get_unrendered_local_toctree(app, pagename, startdepth, **kwargs)
ancestorname, toctree_obj = _get_ancestor_pagename(
app=app, pagename=pagename, startdepth=startdepth
)
if ancestorname is None:
return True # suppress
if kwargs.get("includehidden", False):
# if ancestor is found and `includehidden=True` we're guaranteed there's a
# TocTree to be shown, so don't suppress
return False

# we've found an ancestor page, but `includehidden=False` so we can't be sure if
# there's a TocTree fragment that should be shown on this page; unfortunately we
# must resolve the whole TOC subtree to find out
toctree = get_nonroot_toctree(

Check warning on line 82 in src/pydata_sphinx_theme/toctree.py

View check run for this annotation

Codecov / codecov/patch

src/pydata_sphinx_theme/toctree.py#L82

Added line #L82 was not covered by tests
app, pagename, ancestorname, toctree_obj, **kwargs
)
return toctree is None

Check warning on line 85 in src/pydata_sphinx_theme/toctree.py

View check run for this annotation

Codecov / codecov/patch

src/pydata_sphinx_theme/toctree.py#L85

Added line #L85 was not covered by tests

@cache
def get_or_create_id_generator(base_id: str) -> Iterator[str]:
Expand Down Expand Up @@ -118,6 +116,9 @@
if sphinx.version_info[:2] >= (7, 2):
from sphinx.environment.adapters.toctree import _get_toctree_ancestors

# NOTE: `env.toctree_includes` is a dict mapping pagenames to any (possibly
# hidden) TocTree directives on that page (i.e., the "child" pages nested
# under `pagename`).
active_header_page = [
*_get_toctree_ancestors(app.env.toctree_includes, pagename)
]
Expand All @@ -127,14 +128,18 @@
# The final list item will be the top-most ancestor
active_header_page = active_header_page[-1]

# Find the root document because it lists our top-level toctree pages
root = app.env.tocs[app.config.root_doc]
# NOTE: `env.tocs` is a dict mapping pagenames to hierarchical bullet-lists
# ("nodetrees" in Sphinx parlance) of in-page headings (including `toctree::`
# directives). Thus the `tocs` of `root_doc` yields the top-level pages that sit
# just below the root of our site
root_toc = app.env.tocs[app.config.root_doc]

# Iterate through each toctree node in the root document
# Grab the toctree pages and find the relative link + title.
links_html = []
# TODO: use `root.findall(TocTreeNodeClass)` once docutils min version >=0.18.1
for toc in traverse_or_findall(root, TocTreeNodeClass):
# Iterate through each node in the root document toc.
# Grab the toctree pages and find the relative link + title.
for toc in traverse_or_findall(root_toc, TocTreeNodeClass):
# TODO: ↑↑↑ use `root_toc.findall(TocTreeNodeClass)` ↑↑↑
# once docutils min version >=0.18.1
for title, page in toc.attributes["entries"]:
# if the page is using "self" use the correct link
page = toc.attributes["parent"] if page == "self" else page
Expand Down Expand Up @@ -262,17 +267,27 @@
kind : "sidebar" or "raw". Whether to generate HTML meant for sidebar navigation ("sidebar") or to return the raw BeautifulSoup object ("raw").
startdepth : The level of the toctree at which to start. By default, for the navbar uses the normal toctree (`startdepth=0`), and for the sidebar starts from the second level (`startdepth=1`).
show_nav_level : The level of the navigation bar to toggle as visible on page load. By default, this level is 1, and only top-level pages are shown, with drop-boxes to reveal children. Increasing `show_nav_level` will show child levels as well.
kwargs: passed to the Sphinx `toctree` template function.
kwargs : passed to the Sphinx `toctree` template function.

Returns:
HTML string (if kind == "sidebar") OR BeautifulSoup object (if kind == "raw")
"""
if startdepth == 0:
html_toctree = context["toctree"](**kwargs)

Check warning on line 276 in src/pydata_sphinx_theme/toctree.py

View check run for this annotation

Codecov / codecov/patch

src/pydata_sphinx_theme/toctree.py#L276

Added line #L276 was not covered by tests
else:
# find relevant ancestor page; some pages (search, genindex) won't have one
ancestorname, toctree_obj = _get_ancestor_pagename(
app=app, pagename=pagename, startdepth=startdepth
)
if ancestorname is None:
raise RuntimeError(

Check warning on line 283 in src/pydata_sphinx_theme/toctree.py

View check run for this annotation

Codecov / codecov/patch

src/pydata_sphinx_theme/toctree.py#L283

Added line #L283 was not covered by tests
"Template requested to generate a TocTree fragment but no suitable "
"ancestor found to act as root node. Please report this to theme "
"developers."
)
# select the "active" subset of the navigation tree for the sidebar
toctree_element = get_unrendered_local_toctree(
app, pagename, startdepth, **kwargs
toctree_element = get_nonroot_toctree(
app, pagename, ancestorname, toctree_obj, **kwargs
)
html_toctree = app.builder.render_partial(toctree_element)["fragment"]

Expand Down Expand Up @@ -394,7 +409,7 @@

context["unique_html_id"] = unique_html_id
context["generate_header_nav_html"] = generate_header_nav_html
context["missing_sidebar_toctree"] = missing_sidebar_toctree
context["suppress_sidebar_toctree"] = suppress_sidebar_toctree
context["generate_toctree_html"] = generate_toctree_html
context["generate_toc_html"] = generate_toc_html
context["navbar_align_class"] = navbar_align_class
Expand Down Expand Up @@ -459,36 +474,54 @@
element.insert(1, checkbox)


def get_local_toctree_for_doc(
toctree: TocTree, indexname: str, pagename: str, builder, collapse: bool, **kwargs
) -> List[BeautifulSoup]:
"""Get the "local" TocTree containing `pagename` rooted at `indexname`.

The Sphinx equivalent is TocTree.get_toctree_for(), which always uses the "root"
or "global" TocTree:

doctree = self.env.get_doctree(self.env.config.root_doc)

Whereas here we return a subset of the global toctree, rooted at `indexname`
(e.g. starting at a second level for the sidebar).
def get_nonroot_toctree(
app: Sphinx, pagename: str, ancestorname: str, toctree, **kwargs
):
"""Get the partial TocTree (rooted at `ancestorname`) that dominates `pagename`.

Parameters:
app : Sphinx app.
pagename : Name of the current page (as Sphinx knows it; i.e., its relative path
from the documentation root).
ancestorname : Name of a page that dominates `pagename` and that will serve as the
root of the TocTree fragment.
toctree : A Sphinx TocTree object. Since this is always needed when finding the
ancestorname (see _get_ancestor_pagename), it's more efficient to pass it here to
re-use it.
kwargs : passed to the Sphinx `toctree` template function.

This is similar to `context["toctree"](**kwargs)` (AKA `toctree(**kwargs)` within a
Jinja template), or `TocTree.get_toctree_for()`, which always uses the "root"
doctree (i.e., `doctree = self.env.get_doctree(self.env.config.root_doc)`).
"""
partial_doctree = toctree.env.tocs[indexname].deepcopy()

toctrees = []
kwargs.setdefault("collapse", True)
if "maxdepth" not in kwargs or not kwargs["maxdepth"]:
kwargs["maxdepth"] = 0
kwargs["maxdepth"] = int(kwargs["maxdepth"])
kwargs["collapse"] = collapse

# TODO: use `doctree.findall(TocTreeNodeClass)` once docutils min version >=0.18.1
for _node in traverse_or_findall(partial_doctree, TocTreeNodeClass):
# defaults for resolve: prune=True, maxdepth=0, titles_only=False, collapse=False, includehidden=False
_toctree = toctree.resolve(pagename, builder, _node, **kwargs)
if _toctree:
toctrees.append(_toctree)
# starting from ancestor page, recursively parse `toctree::` elements
ancestor_doctree = toctree.env.tocs[ancestorname].deepcopy()
toctrees = []

# for each `toctree::` directive in the ancestor page...
for toctree_node in traverse_or_findall(ancestor_doctree, TocTreeNodeClass):
# TODO: ↑↑↑↑↑↑ use `ancestor_doctree.findall(TocTreeNodeClass)` ↑↑↑↑↑↑
# once docutils min version >=0.18.1
Copy link
Collaborator

@12rambau 12rambau Jan 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# once docutils min version >=0.18.1
# once docutils min version >=0.18.1 (sphinx >=6.2.0)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did you find the info that sphinx 7 is dropping support for docutils 0.18.1? From what I see here sphinx 7.0.1 adds support for docutils 0.20 but there is no mention there of dropping support for older versions. The most recent version bump mentioned is that in sphinx 6.2.0 they dropped support for anything older than docutils 0.18.1.

FWIW if we were to take inspiration from SPEC0 then we would be OK to drop support for docutils 0.18.1 now (as of 2023-12-23) as it is now more than 2 years since its release and there are 3 versions newer than it. On the contrary, SPEC0 would suggest that we support Sphinx 4.4.0 up until yesterday, and support sphinx 4.5.0 through March 2024 (which is more than we currently do)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the pyproject.toml file from Sphinx: https://github.com/sphinx-doc/sphinx/blame/master/pyproject.toml

the current version (>7) is setting docutils to docutils>=0.18.1,<0.21.
using the blame display I checked when the lower bounds was set (sphinx-doc/sphinx@ae9008b) which was released in v6.2.0. So I made a mistake.

# enforce order on the kwargs to ensure caching (this creates an OrderedDict)
drammock marked this conversation as resolved.
Show resolved Hide resolved

# ... resolve that `toctree::` (recursively get children, prune, collapse, etc)
resolved_toctree = toctree.resolve(
docname=pagename,
builder=app.builder,
toctree=toctree_node,
**kwargs,
)
# ... keep the non-empty ones
if resolved_toctree:
toctrees.append(resolved_toctree)
if not toctrees:
return None
# ... and merge them into a single entity
result = toctrees[0]
for toctree in toctrees[1:]:
result.extend(toctree.children)
for resolved_toctree in toctrees[1:]:
result.extend(resolved_toctree.children)

Check warning on line 526 in src/pydata_sphinx_theme/toctree.py

View check run for this annotation

Codecov / codecov/patch

src/pydata_sphinx_theme/toctree.py#L526

Added line #L526 was not covered by tests
return result