Skip to content

[doc] Remove/update some old Sphinx hacks - #24930

Open
tyler-yankee wants to merge 1 commit into
RobotLocomotion:masterfrom
tyler-yankee:sphinx-update-hacks
Open

[doc] Remove/update some old Sphinx hacks#24930
tyler-yankee wants to merge 1 commit into
RobotLocomotion:masterfrom
tyler-yankee:sphinx-update-hacks

Conversation

@tyler-yankee

@tyler-yankee tyler-yankee commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Our minimum-supported Sphinx version is currently 7.2.6, but most of this extension code was written under much older versions (1.* or 2.*). We can take better advantage of many new upstream features.

Towards #24895.


This change is Reviewable

@tyler-yankee tyler-yankee added the release notes: none This pull request should not be mentioned in the release notes label Aug 31, 2026
@tyler-yankee

Copy link
Copy Markdown
Contributor Author

@drake-jenkins-bot linux-noble-unprovisioned-gcc-bazel-experimental-documentation please.

@tyler-yankee
tyler-yankee marked this pull request as ready for review August 31, 2026 17:09

@tyler-yankee tyler-yankee left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

+a:@mwoehlke-kitware for feature review, please.

@tyler-yankee reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware).

@mwoehlke-kitware mwoehlke-kitware left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:lgtm: ...seems okay, I guess? Real test would seem to be what effect (if any) it has on the generated documentation...

@mwoehlke-kitware made 1 comment.
Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers.

@tyler-yankee tyler-yankee left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm a bit concerned that sort_members is in a module called _legacy_class_based as of latest Sphinx (9.1.0), but in 7.2.6 it's not spelled that way, so I think it's fine for now. We'll have to cross that bridge in the future during an upgrade, but IMO this PR is already doing enough. The main goal here is to remove as much old cruft as possible so that the surface area to paw through (and potential failure modes) when fixing the docs for nanobind is less.

I did my best to sanity check the generated documentation locally, but there are probably some edge cases I'm missing where I'll lean on more pydrake-knowledgeable TRI folks. Speaking of which, +a:@rpoyner-tri, would you like to review this one?

@tyler-yankee made 1 comment.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform) (waiting on mwoehlke-kitware and rpoyner-tri).

@tyler-yankee tyler-yankee left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tyler-yankee made 1 comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee rpoyner-tri(platform) (waiting on mwoehlke-kitware, rpoyner-tri, and tyler-yankee).


a discussion (no related file):
working

The new "pybind11_object bases removal" code has the following visual effect:

before:

image.png

after:

image copy 1.png

This seems like an ugly artifact, so I'll look into restoring

Our minimum-supported Sphinx version is currently 7.2.6, but most of
this extension code was written under much older versions (1.* or 2.*).
We can take better advantage of many new upstream features.

* Instead of fully vendoring `add_directive_header`, hook into the
  `autodoc-process-bases` event provided in Sphinx 5.1.
* Instead of fully vendoring `document_members`, target the narrower
  `sort_members` helper which has since been factored out upstream.
* Drop branches that were conditional on older versions of Sphinx
  than our minimum supported.

@tyler-yankee tyler-yankee left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tyler-yankee reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform) (waiting on mwoehlke-kitware and rpoyner-tri).


a discussion (no related file):

Previously, tyler-yankee (Tyler Yankee) wrote…

working

The new "pybind11_object bases removal" code has the following visual effect:

before:

image.png

after:

image copy 1.png

This seems like an ugly artifact, so I'll look into restoring

done

@mwoehlke-kitware mwoehlke-kitware left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@mwoehlke-kitware reviewed 1 file and all commit messages.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform) (waiting on rpoyner-tri).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: none This pull request should not be mentioned in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants