pydata / pydata-sphinx-theme

A clean, three-column Sphinx theme with Bootstrap for the PyData community
https://pydata-sphinx-theme.readthedocs.io
BSD 3-Clause "New" or "Revised" License
607 stars 317 forks source link

[ENH] Allow setting the `startdepth` parameter for the left sidebar #1181

Open leycec opened 1 year ago

leycec commented 1 year ago

Greetings, wonderful theme devs! I'm currently transitioning @beartype's embarrassingly monolithic README.rst documentation ...which is so large it's actually breaking pip-based installation for a subset of users onto ReadTheDocs, backed by this wonderful theme.

Prior theme versions allowed the left sidebar to be "trivially" customized via a _templates/sidebar-nav-bs.html template with horrifying boilerplate I don't pretend to understand like:

<nav class="bd-links d-none d-md-block" id="bd-docs-nav" aria-label="{{ _('Main navigation') }}">
  <div class="bd-toc-item active">
    {{ generate_nav_html("sidebar",
                         startdepth=0,
                         maxdepth=theme_navigation_depth|int,
                         show_nav_level=theme_show_nav_level|int,
                         collapse=theme_collapse_navigation|tobool,
                         includehidden=True,
                         titles_only=True) }}
  </div>
</nav>

Current theme versions instead centralize similar logic inside the main layout.html template. This mostly makes sense, as doing so avoids displaying an empty left sidebar. But this also appears to prevent full-blown customization of the left sidebar (like above), which sorta makes less sense. Specifically, layout.html is now prefaced by this Jinja madness cleverness:

{# Create the sidebar links HTML here to re-use in a few places #}
{# If we have no sidebar links, pop the links component from the sidebar list #}
{%- set sidebar_nav_html = generate_toctree_html("sidebar",
    show_nav_level=theme_show_nav_level|int,
    maxdepth=theme_navigation_depth|int,
    collapse=theme_collapse_navigation|tobool,
    includehidden=True,
    titles_only=True)
-%}
{% if sidebar_nav_html | length == 0 %}
{% set sidebars = sidebars | reject("in", "sidebar-nav-bs.html") | list %}
{% endif %}

My Jinja is weak. Still, I'm pretty sure that precludes meaningful user-driven overrides of the sidebar_nav_html configuration setting – unless I'm missing something, which I probably am. See: "My Jinja is weak."

Superficially, users can (of course) attempt to override sidebar_nav_html by adding a downstream _templates/layout.html file resembling:

{%- extends "!layout.html" %}
{%- set sidebar_nav_html = generate_toctree_html("sidebar",
    startdepth=0,
    show_nav_level=theme_show_nav_level|int,
    maxdepth=theme_navigation_depth|int,
    collapse=theme_collapse_navigation|tobool,
    includehidden=True,
    titles_only=True)
-%}

Pragmatically, that's just a noop. This theme's layout.html already detected sidebar_nav_html as being empty and then forcefully removed sidebar-nav-bs.html from the sidebars list. That behaviour is no longer amenable to user customization. Is that right? If so...

What I Am Begging of You Here is...

A new theme-specific configuration global allowing users to intervene in this process... somehow.

In @beartype's case, I'd just like to display the full TOC tree in the left sidebar by passing startdepth=0 to the generate_toctree_html() call. This appears to be a common concern (e.g., at #90, #221, and presumably elsewhere). Tragically, the internal structure of this theme changed so fundamentally that prior workarounds no longer apply. Thankfully, a permanent fix avoiding fragile external workarounds that were guaranteed to fail (and then did) should be trivial!

Let's add a new theme-specific theme_navigation_start_depth global for parity with existing globals. In theory, adding this single line to layout.html should more-or-less suffice: e.g.,

{# Create the sidebar links HTML here to re-use in a few places #}
{# If we have no sidebar links, pop the links component from the sidebar list #}
{%- set sidebar_nav_html = generate_toctree_html("sidebar",
    startdepth=theme_navigation_start_depth|int,  # <-- MAGIC ONE-LINER IS MAGICAL.
    show_nav_level=theme_show_nav_level|int,
    maxdepth=theme_navigation_depth|int,
    collapse=theme_collapse_navigation|tobool,
    includehidden=True,
    titles_only=True)
-%}
{% if sidebar_nav_html | length == 0 %}
{% set sidebars = sidebars | reject("in", "sidebar-nav-bs.html") | list %}
{% endif %}

If everyone's amenable, I'd be happy to submit a working PR with working tests (and possibly even documentation – but let us not get too optimistic here) at some point. Until then, I've very reluctantly pinned @beartype to a prior version of this theme. (The RTD flyout disappearing under recent versions also made this decision easier than I would have liked. I sigh.)

Thanks again for all the wondrous theming, everyone. PyData FTW forever! :+1:

SimenZhor commented 1 year ago

I was coincidentially struggling with this exact problem right now myself as I came across this issue. I tested your solution locally and it's exactly what I was looking for. The only thing I'd like to add is that the title in sidebar-nav-bs.html ("Section Navigation") should probably also be made configurable with the introduction of this theme parameter.

Alternatively it could be automatically changed to something like "Site Navigation" when using "navigation_start_depth": 0

As a user, I'm giving a huge thumbs up to this suggestion 👍

drammock commented 1 year ago

@choldgraf a global config for TOC startdepth seems pretty harmless here and would grant a wish that several users have pined for. WDYT? @12rambau any strong feelings here?

crossref to #944

SimenZhor commented 1 year ago

@leycec I mentioned yesterday that I tested your proposed solution by locally modifying the extension itself, which obviously would be a bad idea for a pipeline build. But building on your template idea, I've found another way to solve this issue that should work with pipeline builds too. I thought I'd share it as a temporary workaround while the maintainers discuss whether they agree to add this feature. I'll write it as a step-by-step guide that can be used by others too.

  1. I created a _templates folder inside my docs/source directory (reference)
  2. I made a file inside that directory called sidebar-nav-bs-override.html sidebar-nav-bs.html with the following content:
    
    {%- set sidebar_nav_html = generate_toctree_html("sidebar",
    startdepth=0,
    show_nav_level=theme_show_nav_level|int,
    maxdepth=theme_navigation_depth|int,
    collapse=theme_collapse_navigation|tobool,
    includehidden=True,
    titles_only=True)
    -%}
~~3. In my conf.py file, I've added the following entry (note that this should be outside the `html_theme_options` dict):~~ 
```Python
html_sidebars = {
    "**": ["sidebar-nav-bs-override"], # Use the newly made template instead of the default sidebar navigation
    "index": [] # Don't show sidebar on frontpage
}

EDIT: updated to @drammock 's improved implementation from the next comment

drammock commented 1 year ago

@SimenZhor it actually could be even easier; if you name your template sidebar-nav-bs.html (omit the override) then it will automatically override the built-in template without needing to change anything in conf.py. This feature is built-in to Sphinx as part of theme inheritance. But still, I think we should provide the config var.

leycec commented 1 year ago

@SimenZhor: Such utter cleverness could have only come from the nation that invented black metal. Head-banging Canadians everywhere approve. :man_singer:

@drammock: Actually, @SimenZhor maybe has the right of it... maybe. By defining a completely new sidebar orthogonal to the existing sidebar-nav-bs, he's circumventing this pernicious logic in PyData's root layout.html template that forcibly removes sidebar-nav-bs from the sidebars list before downstream projects like ours manage to have a say:

{% if sidebar_nav_html | length == 0 %}
{% set sidebars = sidebars | reject("in", "sidebar-nav-bs.html") | list %}
{% endif %}

Of course, my Jinja is weak, I haven't actually tested anything anyone said, and it is currently hailing golf ball-sized ice chunks outside that I'm doing my best to ignore. @beartype needed this to work yesterday. The easiest means of accomplishing that was to roll back time itself to pydata-sphinx-theme 0.7.2 – the theme version so old that even Sphinx muttered "Get uff muh lawn!" when I pinned it there.

But... yeah. The maddening intensity of this discussion corroborates @drammock's concluding remarks of timeless wisdom:

But still, I think we should provide the config var.

These words are like sweet raindrops on my brain.

SimenZhor commented 1 year ago

@drammock Nice! That's even better of course. I've updated my original comment as that may be a very useful trick for other overrides too.

But still, I think we should provide the config var.

I totally agree, that would be way cleaner.

@leycec the implementation @drammock suggests does indeed work. Keep in mind that we're not overriding the main layout.html here. @drammock suggests to skip the entire override aspect and that we instead redefine the entire sidebar-nav-bs.html file. In that new file we're also redefining (and using) the sidebar_nav_html variable, as you originally suggested. With drammocks redefinition, the sidebar-nav-bs entry is always present in the sidebars dict (called html_sidebars in conf.py) as the default value, so the jinja-code you're referencing doesn't reject the html template file.

leycec commented 1 year ago

:exploding_head:

drammock commented 1 year ago

If everyone's amenable, I'd be happy to submit a working PR with working tests (and possibly even documentation – but let us not get too optimistic here) at some point

@leycec I just noticed this volunteering, which is great because I'm (mostly) off this week for moving house, and ran out of my limited time today to do a proper job of this (viz, testing and docs; as you note the code change is trivial). I'll unassign myself, and if you don't get it done, I can pick up where you leave off next week.

leycec commented 1 year ago

Wondrous! Although I promise nothing and will surely deliver even less, I'll heartily endeavour to do something... at some point.

The current low-hanging fruit for me is to finish transitioning @beartype's documentation onto ReadTheDocs (RTD) using an ancient Sphinx configuration backed by pydata-sphinx-theme 0.7.2 :scream: I found lying around somewhere. That's currently blocking @beartype installation for a subset of users, which is as bad as gets. Until I crunch that out of the park, my open-source volunteer hands are a bit hog-tied. Let's see who free time materializes for first. :smile_cat:

SimenZhor commented 1 year ago

@drammock, @leycec, I've made an implementation for this and opened PR #1241 for feedback.

(It's still missing documentation for the new parameters)

kathatherine commented 1 year ago

Just wanted to add my own request for this to be looked at again. The main theme works amazingly for our main documentation site, but we have two sub-projects that could really use a main page sidebar for top level TOC items. I'm attempting to use the workaround above, but having no luck, since my theme customization experience is nearly zero. Would be so cool to have this so my docs are able to have consistent theming. I'm gonna keep banging my head against the workaround for a while, though, because I love this theme. Thanks!

Edit: Of course as soon as I say that, I do get the workaround above to work. Using sidebar-nav-bs-override.html and then forcing it via the conf.py was the answer for me, so a +1 for that method.

leycec commented 1 year ago

@kathatherine: Consider also migrating from this lower-level theme to the higher-level sphinx-book-theme, which internally depends on this theme but forcefully enables a main page sidebar for top-level TOC items in the way that everybody expects and wants.

Personally, I hit the same conundrum that you did. I may grok HTML and CSS, but I'd rather not squander precious unpaid open-source time on fighting Sphinx themes over HTML and CSS. Instead, I just migrated from this theme to sphinx-book-theme. Instant win! Unsurprisingly, sphinx-book-theme is also the official theme for all Jupyter documentation... Yeah. Jupyter. :partying_face:

12rambau commented 1 year ago

Hi @leycec I understand that you are frustrated that your issue is not yet solved. Note that some other are also waiting for solutions for extremely large project like scikitlearn (yes @tupui I didn't forget I just cannot find the time as I changed job). This theme is maintained by benevolent contributors and we would be more than happy to count you among us.

pydata-sphinx-theme is not by any mean a "low-level" theme. It's just designed for extremely large API (initially pandas) that benefit from the navbar AND left sidebar to avoid overloading their readers. So no, what you ask for is not "what everybody expects and want", it's just the classic way of using Sphinx which is why we advertise sphinx-book-theme and furo in our landing documentation https://pydata-sphinx-theme.readthedocs.io/en/stable/. Note that jupyter (https://jupyterlab.readthedocs.io/en/latest/, https://docs.jupyter.org/en/latest/use/using.html) is using PST as it suits their needs for technical documentation.

We work on issues, but it's hard to keep up.

Pierrick Rambaud Contributor to PST that uses his precious unpaid open-source time to work on sphinx tools instead of doing what he loves: GIS analysis.