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

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

Open
leycec opened this issue Feb 15, 2023 · 13 comments · May be fixed by #1241
Open

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

leycec opened this issue Feb 15, 2023 · 13 comments · May be fixed by #1241
Assignees

Comments

@leycec
Copy link

leycec commented Feb 15, 2023

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! 👍

leycec added a commit to beartype/beartype that referenced this issue Feb 15, 2023
This commit is the next in a commit chain coercing our monolithic
`README.rst` documentation onto Read the Docs (RTD), en-route to
resolving issue #203 kindly submitted by @LittleBigGene (AKA the dynamo
of the cell). Specifically, this commit circumvents upstream theme
issues pydata/pydata-sphinx-theme#90, pydata/pydata-sphinx-theme#221,
and pydata/pydata-sphinx-theme#1181 with the "standard"
`_templates/sidebar-nav-bs.html` hack shamelessly copy-pasted into
literally *every* project requiring that theme. This includes @beartype,
because why not spew boilerplate that nobody understands everywhere?
Sadly, doing so now requires pinning to a maximum obsolete version of
this theme that will also surely die soon. And this is why I facepalm.
(*Illogical ontological topology!*)
@SimenZhor
Copy link

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
Copy link
Collaborator

drammock commented Feb 15, 2023

@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
Copy link

SimenZhor commented Feb 16, 2023

@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)
-%}

<nav class="bd-links" id="bd-docs-nav" aria-label="{{ _('Section navigation') }}">
  <p class="bd-links__title" role="heading" aria-level="1">
    {{ _("Site Navigation") }}
  </p>
  <div class="bd-toc-item navbar-nav">
    {{ sidebar_nav_html }}
  </div>
</nav>

3. In my conf.py file, I've added the following entry (note that this should be outside the html_theme_options dict):

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
Copy link
Collaborator

@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
Copy link
Author

leycec commented Feb 17, 2023

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

@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
Copy link

SimenZhor commented Feb 17, 2023

@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
Copy link
Author

leycec commented Feb 17, 2023

🤯

@drammock drammock self-assigned this Feb 17, 2023
@drammock
Copy link
Collaborator

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.

@drammock drammock assigned leycec and unassigned drammock Feb 22, 2023
@leycec
Copy link
Author

leycec commented Feb 22, 2023

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 😱 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. 😸

SimenZhor added a commit to SimenZhor/pydata-sphinx-theme that referenced this issue Mar 11, 2023
Allows users to start the sidebar Toc at level 0

Addresses issue pydata#1181
@SimenZhor
Copy link

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

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

leycec added a commit to beartype/beartype that referenced this issue Apr 8, 2023
This minor release delivers pulse-quickening support for **pandera
(pandas) type hints,** **PEP 484,** **PEP 585**, **PEP 591**, **PEP
647**, **PEP 3119**, and **pseudo-callables.**

This minor release resolves **12 issues** and merges **2 pull
requests.** But first, a quiet word from our wondrous sponsors. They are
monocled QA wizards who serve justice while crushing bugs for humanity.
High fives, please!

## Beartype Sponsors

* [**ZeroGuard:** The Modern Threat Hunting
  Platform](https://zeroguard.com). *All the signals, All the time.*

Thunderous applause echoes through the cavernous confines of the Bear
Den. 👏 🐻‍❄️ 👏

And now... the moment we've waited for. A heinous display of plaintext
that assaults all five senses simultaneously.

## Compatibility Added

* **Pandera (pandas) type hints** (i.e., ad-hoc PEP-noncompliant type
  hints validating pandas `DataFrame` objects, produced by subscripting
  factories published by the `pandera.typing` subpackage and validated
  *only* by user-defined callables decorated by the ad-hoc
  PEP-noncompliant `@pandera.check_types` runtime type-checking
  decorator), resolving feature request #227 kindly submitted by
  @ulfaslakprecis (Ulf Aslak) the Big Boss Typer. @beartype now:
  * Transparently supports pandera's PEP-noncompliant
    `@pandera.check_types` decorator for deeply runtime type-checking
    arbitrary pandas objects.
  * *Always* performs a rudimentary `O(1)` `isinstance()`-based
    type-check for each Pandera type hint. Doing so substantially
    improves usability in common use cases, including:
    * Callables annotated by one or more pandera type hints that are
      correctly decorated by @beartype but incorrectly *not* decorated
      by the pandera-specific `@pandera.check_types` decorator.
    * (Data)classes annotated by one or more pandera type hints.
    * Pandera type hints passed as the second argument to
      statement-level @beartype type-checkers – including:
      * `beartype.door.is_bearable()`.
      * `beartype.door.die_if_unbearable()`.
  * Implements a non-trivial trie data structure to efficiently
    detect all type hints produced by subscriptable factories in the
    `pandera.typing` submodule. Let us pretend this never happened,
    @ulfaslakprecis.
* **PEP 484- and 585-compliant generator constraints.** This release
  relaxes prior constraints erroneously imposed by @beartype
  prohibiting both asynchronous and synchronous generator callables from
  being annotated as returning unsubscripted standard abstract base
  classes (ABCs) defined by the `collections.abc` module. Now, @beartype
  permits:
  * Asynchronous generator callables to be annotated as returning the
    unsubscripted `collections.abc.AsyncGenerator` type.
  * Synchronous generator callables to be annotated as returning the
    unsubscripted `collections.abc.Generator` type.
* **PEP 591** (i.e., `typing.Final[...]` type hints), partially
  resolving issue #223 kindly submitted by the acronym known only as
  @JWCS (Jude). @beartype now trivially reduces *all*
  `typing.Final[{hint}]` type hints to merely `{hint}` (e.g.,
  `typing.Final[int]` to `int`). In other words, @beartype no longer
  raises exceptions when confronted with final type hints and instead at
  least tries to do the right thing. This still isn't *quite* what
  everyone wants @beartype to do here; ideally, @beartype should also
  raise exceptions on detecting attempts to redefine instance and class
  variables annotated as `Final[...]`. Doing so is *definitely* feasible
  and exactly what @beartype should *eventually* do – but also
  non-trivial, because whatever @beartype *eventually* does needs to
  preserve compatibility with all implementations of the `@dataclass`
  decorator across all versions of Python now and forever. Cue that
  head-throbbing migraine. It's comin'! Oh, I can feel it!
* **PEP 647** (i.e., `typing.TypeGuard[...] type hints`), resolving
  feature request #221 kindly submitted by Google X researcher
  extraordinaire @patrick-kidger. @beartype now trivially reduces *all*
  `typing.TypeGuard[...]` type hints to the builtin `bool` type.

## Compatibility Improved

* **PEP 3119.** @beartype now detects both
  **non-isinstanceable classes** (i.e., classes whose metaclasses define
  PEP 3119-compliant `__instancecheck__()` dunder methods
  unconditionally raising `TypeError` exceptions) and
  **non-issubclassable classes** (i.e., classes whose metaclasses define
  PEP 3119-compliant `__subclasscheck__()` dunder methods
  unconditionally raising `TypeError` exceptions) more narrowly for
  safety, resolving issue #220 kindly submitted by *ex*traordinary
  Google X researcher @patrick-kidger (Patrick Kidger). Notably,
  @beartype now *only* accepts `TypeError` exceptions as connoting
  non-isinstanceability and non-issubclassability. Previously, @beartype
  broadly treated any class raising any exception whatsoever when passed
  as the second parameter to `isinstance()` and `issubclass()` as
  non-isinstanceable and non-issubclassable. Sadly, doing so erroneously
  raises false positives for isinstanceable and issubclassable
  metaclasses that have yet to be fully "initialized" at the early time
  the `@beartype` decorator performs this detection.

## Features Added

* **Pseudo-callable monkey-patching support.** `@beartype` now supports
  **pseudo-callables** (i.e., otherwise uncallable objects masquerading
  as callable by defining the `__call__()` dunder method), resolving
  feature request #211 kindly submitted by Google X typing guru
  @patrick-kidger (Patrick Kidger). When passed a pseudo-callable whose
  `__call__()` method is annotated by one or more type hints,
  `@beartype` runtime type-checks that method in the standard way.

## Documentation Revised

* **Literally everything,** also known as the release that migrated
  `README.rst` -> [Read the Docs
  (RtD)](https://beartype.readthedocs.io), resolving both issue #203
  kindly submitted by @LittleBigGene (AKA the dynamo of the cell) and
  ancient issue #8 kindly submitted by @felix-hilden (AKA the Finnish
  computer vision art genius that really made all of this possible).
  Readable documentation slowly emerges from the primordial soup of
  @beartype's shameless past for which we cannot be blamed. @leycec was
  young and "spirited" back then. Specifically, this release:
  * Coerces our prior monolithic slab of unreadable `README.rst`
    documentation into a website graciously hosted by Read the Docs
    (RtD) subdividing that prior documentation into well-structured
    pages, resolving issue #203 kindly submitted by @LittleBigGene (AKA
    the dynamo of the cell).
  * Documents *most* previously undocumented public APIs in the
    @beartype codebase. Although a handful of public APIs remain
    undocumented (notably, the `beartype.peps` submodule), these
    undocumented APIs are assumed to either be sufficiently unpopular or
    non-useful to warrant investing additional scarce resources here.
  * Updates our installation instructions to note @beartype's recent
    availability as official packages in the official package
    repositories of various Linux distributions. Truly, this can only be
    the final mark of pride. These include:
    * Gentoo Linux's Portage tree.
    * Arch Linux's Arch User Repository (AUR).
  * Improves the Python code sample embedded in the ["Are We on the
    Worst Timeline?" subsection of our **Beartype Errors**
    chapter](https://beartype.readthedocs.io/en/latest/api_roar/#are-we-on-the-worst-timeline).
    Thanks to @JWCS for their related pull request (PR) #210, which
    strongly inspired this bald-faced improvement to the usability of
    our `beartype.typing` API.
  * Circumvents multiple long-standing upstream issues in the PyData
    Sphinx theme regarding empty left sidebars via the requisite
    `_templates/sidebar-nav-bs.html` template hack shamelessly
    copy-pasted into literally *every* project requiring this theme.
    This includes @beartype, because why not spew boilerplate that
    nobody understands everywhere? Sadly, doing so requires pinning to a
    maximum obsolete version of this theme that will surely die soon.
    And this is why I facepalm. These issues include:
    * pydata/pydata-sphinx-theme#90.
    * pydata/pydata-sphinx-theme#221.
    * pydata/pydata-sphinx-theme#1181.
  * Truncates our `README.rst` documentation to a placeholder stub that
    just directs everyone to RtD instead.
  * Improves `linecache` integration commentary. Specifically, a pull
    request by @faangbait (AKA the little-known third member of Daft
    Punk) improves internal commentary in our private
    `beartype._util.func.utilfuncmake.make_func()` factory function
    responsible for dynamically synthesizing new in-memory functions
    on-the-fly. Our suspicious usage of `None` as the second item of
    tuples added as values to the standard `linecache.cache` global
    dictionary has now been documented. Thanks so much for this
    stupendous contribution, @faangbait!

## Tests Improved

* **Mypy integration.** This release improves our `test_pep561_mypy()`
  integration test to intentionally ignore unhelpful non-fatal warnings
  improperly emitted by mypy (which encourage usage of
  `typing_extensions`, oddly enough).
* **Sphinx integration.** This release resolves multiple intersecting
  issues involving integration testing of Sphinx + @beartype, including:
  * `test_beartype_in_sphinx()` h0tfix is h0t. This release generalizes
    our test-specific `test_beartype_in_sphinx()` integration test to
    support arbitrary versions of Sphinx, resolving issue #209 kindly
    submitted by @danigm the sun-loving Málaga resident who frolics in
    the sea that Canadians everywhere are openly jealous of.
    Specifically, this release fundamentally refactors this integration
    test to fork a new Python interpreter as a subprocess of the current
    `pytest` process running the `sphinx-build` command.
  * A Python 3.7-specific failure in our continuous integration (CI)
    workflow caused by Sphinx attempting to call deprecated
    functionality of the third-party `pkg_resources` package. This
    release simply avoids installing Sphinx entirely under Python 3.7;
    although admittedly crude, it's unclear how else @beartype could
    possibly resolve this. Since Python 3.7 has almost hit its official
    End-Of-Life (EOL) and thus increasingly poses a security concern,
    this is hardly the worst resolution ever. Really! Believe what we're
    saying.

Break nothing! It's the @beartype way. This is why @leycec cries like a
mewling cat with no milk. (*Thrilling chills spill towards an untoward ontology!*)
@kathatherine
Copy link

kathatherine commented Jun 28, 2023

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
Copy link
Author

leycec commented Jun 30, 2023

@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. 🥳

@12rambau
Copy link
Collaborator

12rambau commented Jul 2, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants