Incompatibility with Sphinx dev

[ayshih]

A Sphinx PR merged today (sphinx-doc/sphinx#11565) renamed TocTree.get_toctree_ancestors() to TocTree._get_toctree_ancestors(). This causes this theme to error here:

pydata-sphinx-theme/src/pydata_sphinx_theme/toctree.py

Line 420 in 0956f22

ancestors = toctree.get_toctree_ancestors(pagename)

I don't know whether you want to ask Sphinx if they intended to make that change in API, or you just want to work around it in the theme.

[drammock]

Thanks for the heads-up; I'll raise this upstream. I guess we should be more diligent about testing against dev version of sphinx.

[AA-Turner]

In general, we don't consider TocTree to be public API. I'd be interested to know what you use it for, perhaps we could find or create a public API that you could use?

A

[drammock]

a lot of the TocTree code was written by @choldgraf I think, so I'd have to dig to see exactly how we use it everywhere. But regarding this particular case (TocTree.get_toctree_ancestors) there are two uses:

1. generate a sub-TocTree for each second-level page and its children --- this theme puts first-level pages in a topbar navigation, and sub-TocTrees in the left sidebar (so the sidebar-displayed TocTree depends on which page you happen to be viewing).

pydata-sphinx-theme/src/pydata_sphinx_theme/toctree.py

Line 420 in 0956f22

ancestors = toctree.get_toctree_ancestors(pagename)

which is called from

pydata-sphinx-theme/src/pydata_sphinx_theme/toctree.py

Line 181 in 0956f22

toc_sphinx = index_toctree(app, pagename, startdepth, **kwargs)

2. figure out which of the top-level headings the current page falls under (if any --- orphans, main index.html, and possibly others like search or genindex will not) so that we can style that top-level heading in the UI:

pydata-sphinx-theme/src/pydata_sphinx_theme/toctree.py

Lines 64 to 69 in 0956f22

	# Find the active header navigation item so we decide whether to highlight
	# Will be empty if there is no active page (root_doc, or genindex etc)
	active_header_page = toctree.get_toctree_ancestors(pagename)
	if active_header_page:
	# The final list item will be the top-most ancestor
	active_header_page = active_header_page[-1]

[pradyunsg]

I'm doing some similar things in https://github.com/pradyunsg/lutra/blob/97f6da4f24ab3a76fdbb18e74093b69176a89f5b/src/lutra/_navigation.py with TocTree -- where I'm using the TocTree object to get the actual toctree described for the documentation; and process that to pick up on captions, as well as the heirarchy of pages. If a usable API providing that information was designed, it would be good to have sphinx-doc/sphinx#10697 catered to as well! :)

In general, we don't consider TocTree to be public API.

What is that based on? The name is public by all Python naming conventions and https://www.sphinx-doc.org/en/master/internals/release-process.html#deprecation-policy doesn't really distinguish between APIs on some external factor (eg: whether something is documented), and past changes in Sphinx have followed the conservative deprecation policy around these sorts of API renames (eg: sphinx-doc/sphinx@77a02cf#diff-f41e19a9b77f4670d4dd1ea61f80d518c93a4403112fd7e91253dac47cf55fe5R345).

[AA-Turner]

What is that based on?

It is entirely undocumented (see the Extension API reference). Maintainers of Sphinx (and Docutils) have historically not marked things as private with the leading underscore convention; I'm trying to clear this up but it takes a lot of time and effort. The conservatism on renamings has been as extension maintainers use undocumented parts of the Sphinx API (likely due to mismatches in expectations viz private/public) and when Sphinx changes or refactors things, there are complaints that X is now broken, perhaps not unreasonably.

Ideally we'd design and document better public APIs, but partly I don't know what the needs are from themes and extensions for said APIs, hence the chicken-and-egg.

A

[pradyunsg]

It is entirely undocumented (see the Extension API reference). Maintainers of Sphinx (and Docutils) have historically not marked things as private with the leading underscore convention;

Yea, that's part of the problem here and the reason that Sphinx should move slowly when making changes IMO. 😅 That said, I empathise that taking that level of care around these things is difficult and (for me as a maintainer) frustratingly slow.

I'm trying to clear this up but it takes a lot of time and effort.

This is very much appreciated!

Ideally we'd design and document better public APIs, but partly I don't know what the needs are from themes and extensions for said APIs, hence the chicken-and-egg.

In this case, we have the expressed need described and presented to the Sphinx maintainers in the issue tracker and mentioned the upcoming changes as well.

Is there anything else that we can do to make this easier for you?

[CAM-Gerlach]

To note, this style of putting the inter-document toctree in the left sidebar and the intra-document toctree in the right sidebar has emerged as the de-facto standard in most modern documentation, and is implemented by the main popular themes. Perhaps it would be worth designing some type of higher-order mechanism to do this? Though, of course, if that's non-trivial it shouldn't block adding some form of public API that themes can use for this in the meantime.

[pradyunsg]

My 2 cents is that it'd be better for Sphinx to expose the toctree structure to themes such that they can do the code generation, instead of one-more-option for the tab-style layouts, that provides more flexibility and doesn't require changes in Sphinx as the documentation site conventions/expectations evolve.

[pllim]

Sphinx 7.2 was released last night. So now this theme breaks in latest stable Sphinx. Any project using this theme have to pin sphinx<7.2, which isn't pretty.

[AA-Turner]

xref #1406 (comment)

[drammock]

@AA-Turner we certainly appreciate you submitting a fix here, but unfortunately that doesn't fully solve the problem because we have a couple of other blocker issues that are preventing us from pushing out a release on Sphinx's schedule.

[pllim]

@drammock , not even a hot fix?

[AA-Turner]

@pllim please may you open a new Sphinx issue? 7.2.2 may be needed...

A

[AA-Turner]

Just the above would be fine initially, mainly so I don't forget the new issue.

[pllim]

Ah, wait. I think the warnings are from a different upstream fix (glue-core) that was released right after your release. I apologize for the noise...

[AA-Turner]

This can probably be closed now.

A