Skip to content

[docs] Fix Sphinx issue with linkless summaries#3284

Merged
johnkerl merged 4 commits intomainfrom
kerl/fix-sphinx-linkless-summaries
Nov 5, 2024
Merged

[docs] Fix Sphinx issue with linkless summaries#3284
johnkerl merged 4 commits intomainfrom
kerl/fix-sphinx-linkless-summaries

Conversation

@johnkerl
Copy link
Copy Markdown
Contributor

@johnkerl johnkerl commented Nov 2, 2024

Issue and/or context: Ongoing work as part of #3282. Also fixes #3227.

Problem

What's right

Screenshot 2024-11-02 at 1 21 16 PM

Screenshot 2024-11-02 at 1 21 32 PM

What's wrong

  • Simply go one level deeper: scroll down to "Methods"
  • Hover over a method, e.g. add_new_collection
  • This is not a hyperlink
  • What you see is the first sentence of its docstring
  • Nothing you can click on will show you the rest of the docstring

Screenshot 2024-11-02 at 1 24 04 PM

Analysis

At the first level we have hand-written .rst like this:

We keystroke out

.. autosummary::
    :toctree: _autosummary/

At the next level, we're letting autogen recurse for us and create .rst files. The .rst files that are auto-generated are missing this line:

    :toctree: _autosummary/

I spent the morning debugging, experimenting with sphinx-autogen and sphinx-apidoc, copy/pasting examples out of the Sphinx docs, playing with the :recursive: tag, doing pip install -U of every Sphinx package on my system, etc. -- and I came to the following conclusions:

  • I could not find any syntax that would make the autogenerator emit the :toctree: dirnamegoeshere line on the .rst files it generates.
  • Therefore I autogenerated the "next level in", manually edited those to include the :toctree: dirnamegoeshere lines, and am adding them to source control.

Changes:

As above.

Notes for Reviewer:

This means more manual editng when we create new classes/methods. However, there is already a non-zero amount of manual edits necessary: #3283 is a case in point. I would love to have fully automated/hands-off code -> doc generation but we simply do not have that at present. I will take a method with one more manual step, that produces correct and useful documentation, over a method one with one fewer manual step that produces incomplete and misleading documentation.

To view the docs: please compare

Note this second link is on what readthedocs calls a "hidden branch" which I created as a manual one-off for this PR review.

PR stacking as of today:

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.40%. Comparing base (762abda) to head (00839da).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3284      +/-   ##
==========================================
+ Coverage   85.29%   85.40%   +0.10%     
==========================================
  Files          52       52              
  Lines        5517     5517              
==========================================
+ Hits         4706     4712       +6     
+ Misses        811      805       -6     
Flag Coverage Δ
python 85.40% <ø> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
python_api 85.40% <ø> (+0.10%) ⬆️
libtiledbsoma ∅ <ø> (∅)

Base automatically changed from kerl/readthedocs-pre-1.15 to main November 4, 2024 14:25
@ivirshup
Copy link
Copy Markdown
Collaborator

ivirshup commented Nov 4, 2024

I will try to take a closer look later in the week and see if I can remember how to get autogenerated class docs working. But in the meantime, I noticed that the CSS for the class listing seems to be off in the demo build: https://tiledbsoma.readthedocs.io/en/kerl-fix-sphinx-linkless-summaries/python-tiledbsoma.html#classes

image

@johnkerl
Copy link
Copy Markdown
Contributor Author

johnkerl commented Nov 4, 2024

Thanks @ivirshup !

Can you please be more specific about

CSS for the class listing seems to be off in the demo build:

? I don't know what to try to fix ...

@johnkerl johnkerl force-pushed the kerl/fix-sphinx-linkless-summaries branch from 8a99f68 to 00839da Compare November 5, 2024 16:19
Copy link
Copy Markdown
Contributor

@ryan-williams ryan-williams left a comment

Choose a reason for hiding this comment

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

Approving to unblock, will try to figure out how to get Sphinx to auto-generate the things we want it to, async…

@johnkerl johnkerl merged commit 1b8cd98 into main Nov 5, 2024
@johnkerl johnkerl deleted the kerl/fix-sphinx-linkless-summaries branch November 5, 2024 19:32
@johnkerl
Copy link
Copy Markdown
Contributor Author

johnkerl commented Nov 5, 2024

Approving to unblock, will try to figure out how to get Sphinx to auto-generate the things we want it to, async…

This is great, thanks @ryan-williams !!

Also recall as I noted above, I don't entirely dislike having to spell more things out -- it lets us put methods in more intuitive order.

And honestly, the more I think about it, maybe I don't even really want fully autogenerated docs as much I had (very recently!) thought I did ... having an environment where manual curation is allowed and encouraged, I feel like this will improve the quality of the docs ......

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 this pull request may close these issues.

[Bug] Class methods do not have all desired material in rendered doc pages

3 participants