Skip to content

Use pydata sphinx theme#14477

Closed
saimn wants to merge 7 commits into
astropy:mainfrom
saimn:pydata-sphinx
Closed

Use pydata sphinx theme#14477
saimn wants to merge 7 commits into
astropy:mainfrom
saimn:pydata-sphinx

Conversation

@saimn

@saimn saimn commented Feb 28, 2023

Copy link
Copy Markdown
Contributor

Just a trial to see how it renders on RTD. I tested this a while ago so I don't remember exactly but there are are issues with our docs structure which causes the topbar/left sidebar to contain a lot of sections. So if we want to use this theme we should probably reorganize a it our docs.

Ref #11520.

Description

This pull request is to address ...

Fixes #

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see "When to rebase and squash commits".
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • Is a milestone set? Milestone must be set but we cannot check for it on Actions; do not let the green checkmark fool you.
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@github-actions

Copy link
Copy Markdown
Contributor

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

@saimn

saimn commented Feb 28, 2023

Copy link
Copy Markdown
Contributor Author

Well, bad luck, it seems the last release (from 11 hours ago) is broken (pydata/pydata-sphinx-theme#1220).

@saimn

saimn commented Mar 1, 2023

Copy link
Copy Markdown
Contributor Author

Ok the docs build is here : https://astropy--14477.org.readthedocs.build/en/14477/

To improve the structure I guess we should discuss what should be in the topbar and in the sidebar. Numpy Scipy Pandas uses a similar structure with a few main sections :

Getting started
User Guide
API reference
Development
Release notes

We could do something similar but it requires some massive restructuring of our docs.

@pllim

pllim commented Mar 1, 2023

Copy link
Copy Markdown
Member

I saw this message on their Discord channel:

Quick warning though about using templates. There is an open issue about increased build time of the doc when using templates (it's a problem for big projects like NumPy and SciPy, not so much on small repo and hard to see. But yep we are currently stuck with 0.9 on big repos for that reason.) This appeared on the version 1.10. I could not narrow this down more on the theme side and AFAIK there has not been progress since then.

@pllim

pllim commented Mar 1, 2023

Copy link
Copy Markdown
Member

Thank you so much for spearheading this, @saimn ! I added this to our next dev telecon agenda. 🤞

@tupui

tupui commented Mar 1, 2023

Copy link
Copy Markdown

Hi 👋 I did the work for SciPy (and SALib recently). If you have specific questions, feel free to ping me 😃

@saimn

saimn commented Mar 1, 2023

Copy link
Copy Markdown
Contributor Author

I changed a bit the docs structure based on Numpy/Scipy's model :
https://astropy--14477.org.readthedocs.build/en/14477/
Just a proof of concept, there are many details to solve, where to put pages etc. but at least this gives a better idea.

@mhvk

mhvk commented Mar 2, 2023

Copy link
Copy Markdown
Contributor

Thanks so much, @saimn! I have to admit that on first look, this seems worse than what we have. For one thing not a nice single entry page with most important links accessible (i.e., always will need more clicks to get anywhere). But mostly the actual pages have sidebars and section titles so big that they dominate the content (see below, as I'm not sure it looks the same to everyone). I also note that the nifty >>> for cut-and-paste of just the input is missing.

(Note that I think the numpy documentation looks and feels a bit worse than ours; scipy's is better, so I guess there is room for customization, etc., though that slightly beats the point. I also note that in the scipy examples, the >>> simply cannot get selected, so cut & paste is helped -- though any output is included, so it has to be done in parts.)

Overall, I'm not sure I see a big benefit for what will likely be quite a lot of work...

image

image

@Cadair

Cadair commented Mar 3, 2023

Copy link
Copy Markdown
Member

I think those two screenshots make one massive improvement to the navagability of the docs, which are the really important links currently hidden under "using astropy units" are now clearly accessible from the left sidebar, and the in-page nav is on the right.

This is one of my main gripes when using the astropy docs, that our pattern of putting stuff in a toctree half way down a page and then having a confusing mess of a left sidebar where there's no direct link to those explanation pages sucks.


On another note: SunPy has been looking at themes for a while, and we are currently in a holding pattern for upstream pydata and sphinx-book-theme to update themselves so we can base a new version of our theme on the book theme, just something to think about.

@Cadair

Cadair commented Mar 3, 2023

Copy link
Copy Markdown
Member

Also, I have to disagree with you @mhvk the visual appearance of the page is just a lot better to my eyes, it's cleaner, easier to scan (maybe because there is less stuff there immediately) and looks just a lot less intimidating.

As for the >>> button and other things, they should be reasonably easy to get back. I think there's a sphinx extension package which does that, or maybe it's a theme option.

@tupui

tupui commented Mar 3, 2023

Copy link
Copy Markdown

For >>> do you have the extension sphinx_copybutton? On the theme's documentation it looks like they have the behaviour that you are looking for with the copy button. (I will actually test on SciPy to see if this works.)

@mhvk

mhvk commented Mar 3, 2023

Copy link
Copy Markdown
Contributor

@Cadair - indeed, clearly one looks at different things... Agreed that the sidebars are indeed better, since you now have both the overall outline of the documentation (left) and that of the page (right). I guess I mostly wish those side bars could be adjusted in size or in fact just hidden like is possible with the current docs.

Looking again at the main text, I do like that there are not those extraneous horizontal lines. In the end, I probably mostly dislike the choice of colors (too faint) and font sizes (too big).

Overall, I'm coming around to this!

I guess really my main request is to make sure we avoid pages that are nothing but tables of content, especially for multiple levels. With the sidebar, it would seem fine to just start with a big page like we presently have. (Though not a super big deal: all it would take for me is just change my links to point to the user guide directly rather than to the first page.)

@tupui

tupui commented Mar 3, 2023

Copy link
Copy Markdown

I guess I mostly wish those side bars could be adjusted in size or in fact just hidden like is possible with the current docs.

That could be implemented on the theme side (maybe there is an issue for that, did not check) 👍 The mechanism should actually be there as if you reduce the size of the window for small screens-like phones, the side bars are collapsed elements.

@mhvk

mhvk commented Mar 3, 2023

Copy link
Copy Markdown
Contributor

Sounds good! I'm becoming more and more in favour. There is also a good argument for trying to have the various scientific python packages to have documentation with similar organization.

@saimn

saimn commented Mar 3, 2023

Copy link
Copy Markdown
Contributor Author

I pushed some commits with sphinx_copybutton (we had something custom to hide the >>>, probably directly in the astropy theme) and adding a detailed toc in the index page.

For the index page, initially I was thinking at doing the same thing as Numpy/Scipy, where the index page only has a few boxes with links to the User Guide etc.
If we prefer to have a detailed toc as we have currently, it's probably doable but what I did in my last commit is not ideal. The User Guide sections do not get the toc captions (e.g. "Files, I/O, and Communication"). I will see if I can improve that.

@saimn

saimn commented Mar 3, 2023

Copy link
Copy Markdown
Contributor Author

I cannot include the full toc with captions in the index page or it messes up the navigation :

Screenshot from 2023-03-03 20-03-48

@mhvk

mhvk commented Mar 3, 2023

Copy link
Copy Markdown
Contributor

Maybe it's OK for the main page...

But perhaps what can be changed is to go directly to the full pages for getting started and developer documentation? Both now show TOCs for what in the end are single pages. That may mean some changes in how we handle the fact that the developer documentation is removed from stable versions and just replaced with a link, but hopefully not too hard.

@hamogu

hamogu commented Mar 5, 2023

Copy link
Copy Markdown
Member

I don't have strong feeling either way. Our current docs are not bad, and neither is the new theme. There are going to be some people who like one more vs. the other; I don't think it's a case of "this one is so obviously better that we all agree".

So, to me thee question is more about the effort: How much effort is it to change the theme (beyond this initial proof or concept)? Is extensive rewriting needed? If that's the case, I would argue to stop here, because I don't think the new theme is so obviously better that it's worth it and there are other open issues that we could spend our time on.

On the other hand, is there major work needed with the existing theme that we would save with the new theme (e.g. is is broken on recent version of some major browser)?

To me, simply "unifying the look with other NumFOCUS projects" is not important enough to make the change.

@Cadair

Cadair commented Mar 9, 2023

Copy link
Copy Markdown
Member

In the end, I probably mostly dislike the choice of colors (too faint) and font sizes (too big).

I am pretty sure these things are tweakable without needing to make our own variant of the theme, see: https://pydata-sphinx-theme.readthedocs.io/en/latest/user_guide/styling.html

Our current docs are not bad

In my opinion they certainly aren't good either. As I mentioned in a previous comment I think the navigability of our current / layout + theme leaves a lot to be desired.

the question is more about the effort

The current theme gets older and cruftier by the day, to my knowledge nobody is stepping up to maintain it or update it to newer versions of bootstrap etc. It might not be worth a lot of effort now, but there will come a time.

@mhvk

mhvk commented Mar 9, 2023

Copy link
Copy Markdown
Contributor

I think I'm pretty much sold on this, especially since it seems we have a volunteer to do the work 😺

@pllim

pllim commented Apr 18, 2023

Copy link
Copy Markdown
Member

There are conflicts now. I think we should not touch this until there is consensus to actually do it, and then we should get it done fast and merge, to avoid conflicts like this and wasting dev time.

@pllim

pllim commented May 22, 2023

Copy link
Copy Markdown
Member

I finally have a chance to look into this in detail. And I am wondering if the file location re-org makes sense even if we are not using a new theme. Just moving things around should be uncontroversial, so we can merge that pretty quickly to avoid merge commits. Then the PR to move theme would be much simpler to review. What do you think?

@pllim

pllim commented May 22, 2023

Copy link
Copy Markdown
Member

p.s. Grrr I cannot build the docs locally because sphinx-gallery throwing some remote error. Good thing the RTD preview still works.

@pllim pllim mentioned this pull request May 22, 2023
2 tasks
@drammock

Copy link
Copy Markdown
Contributor

@pllim I took a look at the lack of left sidebar on the main homepage. It is expected behavior of pydata-sphinx-theme (because left sidebar shows "links to any siblings / children of the current active top-level section (corresponding to links in your header navigation bar)"). The main homepage at docs/index is the very top level, which sits outside of any of the top level sections. To force it to be there anyway, I think this will work:

add a new template (maybe called docs/_templates/global_toc.rst) that contains the relevant .. toctree:: directive, and in conf.py add:

html_sidebars = {
    "index": ["global_toc"]
}

because it keeps failing my local build with remote data failure
@drammock

Copy link
Copy Markdown
Contributor

for reducing the file moves: I think you can do this approach:

  1. in docs/index you need a (possibly hidden) TOCTree that only includes the top-level pages for "getting started", "user guide", "development", and "release notes" pages. Those pages should not need to be in child folders.
  2. In each of those top-level section pages, include a (possibly hidden) TOCTree for its children.
  3. For the existing TOCTrees on the main index page, input them as .. contents:: instead of .. toctree:: directives

@drammock

Copy link
Copy Markdown
Contributor

use built-in globaltoc template and style it to remove the bullets:

# conf.py
html_static_path = ["_static"]
html_css_files = ["custom.css"]

html_sidebars = {
    "index": "globaltoc.html"
}
globaltoc_collapse = False  # maybe not needed?
globaltoc_includehidden = True  # probably not needed?
globaltoc_maxdepth = -1  # this isn't working for me, not sure why

in _static/custom.css (to remove bullet symbols):

.sidebar-primary-item > ul > li.toctree-l1 {
    display: inline;
    font-size: larger;
}

@pllim

pllim commented May 25, 2023

Copy link
Copy Markdown
Member

@pllim pllim closed this May 25, 2023
@pllim pllim mentioned this pull request May 25, 2023
@saimn saimn deleted the pydata-sphinx branch September 7, 2023 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants