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

[WIP] [Docs] Docs deps upgrade #39766

Closed

Conversation

peytondmurray
Copy link
Contributor

@peytondmurray peytondmurray commented Sep 20, 2023

WIP PR; here for visibility

Why are these changes needed?

This PR updates the documentation build dependencies to the latest versions. Changes to the docs needed to accommodate this upgrade:

  • Remove margin directives; they are only implemented in sphinx-book-theme, which we are no longer using. These have all been converted to note directives.
  • sphinx-external-toc has been removed. There are a lot of orphaned pages; I've added these to the appropriate pages.
  • Removed LandingPageBG.jpg: the docs landing page intended to use this but has accidentally been targeting LandingPageBG.JPG, and therefore has been failing to fetch the image. I'm just removing it for now, as it doesn't adapt to dark mode anyway.
  • Removed a reference to cluster/kubernetes/troubleshooting/troubleshooting.html#gpu-multitenancy, there is no gpu-multitenancy ref. There are a number of references in notebooks that have been converted to explicit myst references, fixing some issues with reference ambiguity.

Development changes to be addressed before draft is ready for review

  • Makefile changes to speed up development
  • Reenable fail_on_warning in .readthedocs.yml
  • Search for all pngs, investigate to make sure dark-compatible. If incompatible, replace with SVG Update: Partially complete; have finished most critical ones, will defer on rest.
  • CSAT widget
  • Anyscale logo is kind of hard to see on dark background - find another solution Deferred
  • Ask AI widget
  • Example gallery sidebar and searchbar
  • Override pydata-sphinx-theme colors to use colors more in line with Ray branding

Related issue number

Closes #37944.

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@peytondmurray peytondmurray changed the title [WIP] Docs deps upgrade [WIP] [Docs] Docs deps upgrade Sep 20, 2023
@peytondmurray peytondmurray force-pushed the docs-deps-upgrade branch 5 times, most recently from f8acef4 to 9d4af67 Compare September 29, 2023 05:22
@peytondmurray peytondmurray force-pushed the docs-deps-upgrade branch 5 times, most recently from 28f4429 to 9ffb661 Compare October 9, 2023 20:33
doc/Makefile Outdated
@@ -2,7 +2,7 @@
#

# You can set these variables from the command line.
SPHINXOPTS = -a -E
SPHINXOPTS = -W
Copy link
Collaborator

Choose a reason for hiding this comment

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

dummy question, how does this change the build process?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change this back before merging this, but:

  • -a -E makes sphinx rebuild the docs each time instead of using the existing build artifacts
  • -W makes sphinx turn any build warnings into build errors, stopping the build.

@simran-2797
Copy link
Contributor

simran-2797 commented Oct 31, 2023

Few comments from the design side:

  1. I have a design for dark mode for the home page. We can use that - https://www.figma.com/file/fRYp3ZBGbpg9dlu3WlBYY9/Quansight---Example-gallery-and-CSAT?type=design&node-id=288%3A546&mode=design&t=HZiNlsIehbqxIXH5-1
  2. Looks like there are 3 different button states instead of 2 for dark and light
    Screenshot 2023-10-31 at 3 45 17 PM
    Screenshot 2023-10-31 at 3 45 47 PM
    Screenshot 2023-10-31 at 3 46 03 PM
  3. The example gallery is not on a separate page. The filters are missing.
    Screenshot 2023-10-31 at 3 47 08 PM
  4. The search functionality on the example gallery does not work.

@peytondmurray
Copy link
Contributor Author

@simran-2797 Thanks for taking a look. I'll implement the color changes you made in the mockups.

Looks like there are 3 different button states instead of 2 for dark and light

Yes. The tooltip for the widget explains that the switch can be dark, light or use whatever the current system theme is modes. Is this undesirable? It's how all the docs based on pydata-sphinx-theme work.

Re: example gallery - this is something I still need to finish up. Thanks for the reminder!

@angelinalg
Copy link
Contributor

angelinalg commented Oct 31, 2023

  • There is a pound (#) sign that appears at the end of the last paragraph on the RLlib index page that I don't see in master:
Screenshot 2023-10-31 at 4 35 58 PM

@angelinalg
Copy link
Contributor

angelinalg commented Oct 31, 2023

  • Don't forget to turn off the highlighting from search!

@angelinalg
Copy link
Contributor

angelinalg commented Oct 31, 2023

  • Replace Train top page logo with SVG
    Group 48

@peytondmurray
Copy link
Contributor Author

peytondmurray commented Oct 31, 2023

There is a pound (#) sign that appears at the end of the last paragraph on the RLlib index page that I don't see in master

That's because you're hovering over an image that was inserted as a figure. It gives you a way to copy a link to that image; here's the raw rst:

.. figure:: images/rllib-stack.svg
    :align: left
    :width: 650

    **RLlib's API stack:** Built on top of Ray, RLlib offers off-the-shelf, highly distributed
    algorithms, policies, loss functions, and default models (including the option to
    auto-wrap a neural network with an LSTM or an attention net). Furthermore, our library
    comes with a built-in Server/Client setup, allowing you to connect
    hundreds of external simulators (clients) via the network to an RLlib server process,
    which provides learning functionality and serves action queries. User customizations
    are realized via sub-classing the existing abstractions and - by overriding certain
    methods in those sub-classes - define custom behavior.

If this is undesirable behavior, I think there's probably ways of using other directives or turning this feature off. Personally this feature falls in line with behavior seen elsewhere in the docs - for example if you hover over a heading, a hash will appear allowing you to link to that particular section. For that reason, I'd vote to keep it.

@angelinalg
Copy link
Contributor

angelinalg commented Oct 31, 2023

@angelinalg
Copy link
Contributor

angelinalg commented Nov 1, 2023

  • Hide Search Metrics button at the top of the page.
Screenshot 2023-10-31 at 5 03 49 PM

@angelinalg
Copy link
Contributor

angelinalg commented Nov 1, 2023

  • Side nav: Developer Guides should be expandable.
Screenshot 2023-10-31 at 5 17 12 PM

This is what is on master:
Screenshot 2023-10-31 at 5 18 08 PM

@angelinalg
Copy link
Contributor

angelinalg commented Nov 1, 2023

Copy link
Member

@bveeramani bveeramani left a comment

Choose a reason for hiding this comment

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

Data docs overall LGTM. Nice work!

  • Nit: I think we don't want the leading whitespace to be underscored?
image
  • Nit: This is more of a personal preference, but I think the code text is more readable without the grey background.
image
  • When I click "User guides", I don't see the specific user guides in the sidebar. Is this expected? I feel like it's easier to read the sidebar than the list of links in the main page.
image
  • Not sure if this is because we need to merge in master, or because the CSS classes are different, but looks like the tables aren't aligned
image

@peytondmurray
Copy link
Contributor Author

When I click "User guides", I don't see the specific user guides in the sidebar. Is this expected? I feel like it's easier to read the sidebar than the list of links in the main page.

@bveeramani Sidebar behavior is something that I think we haven't yet really figured out, and which isn't well sorted in the current version of the docs. On this branch we have the links defined in sidebar.yml and that's it. Do we want all possible links nested (potentially very deeply) in the sidebar? I'm happy to insert whatever links we want, and at the same time we should also think carefully about how deep nesting affects usability.

Thanks for the feedback!

@bveeramani
Copy link
Member

Do we want all possible links nested (potentially very deeply) in the sidebar?

No, I don't think so. I think the individual API reference pages aren't useful in the sidebar. I think the user guides are helpful, though

@peytondmurray peytondmurray force-pushed the docs-deps-upgrade branch 4 times, most recently from 3409696 to 9eec386 Compare November 7, 2023 00:44
@peytondmurray
Copy link
Contributor Author

Dark mode: Logos don't look good on this page: https://anyscale-ray--39766.com.readthedocs.build/en/39766/train/more-frameworks.html

I look the time to rewrite this: the cards weren't clickable, instead a button was being embedded inside the card leading to some awkward spacing. I added some margin to the tops of the images, this is the result:

image

This looks a little better, but as an alternative we can also replace these with SVGs that have dynamically colored text that can change according to the theme, as we do elsewhere. I think this would be better - what do you think @angelinalg?

@emmyscode
Copy link
Member

emmyscode commented Nov 7, 2023

Review of "Ray Clusters" section

I'm only pointing our discrepancies between the current state and the upgrade, not necessarily sharing an opinion about how the side navigation should work.

  • On the "Ray Clusters Overview" page, the "Learn Key Concepts" button/card link jumps you down past the title on the "Key Concepts" page.
  • On the "Key Concepts" page, each heading (i.e. Ray Cluster, Head Node, Worker Node, Autoscaling, and Ray Jobs) when clicked shoots you up to the Ray Cluster heading.
  • When I'm on a page, it's not highlighted in the sidenav, which causes me to kind of lose where I am, especially in a deeply nested section like "Ray Clusters."
  • When I go to any subpage of "API References" under "Deploying VMs," the sidenav collapses completely and I lose my spot.
  • "API References" under "Deploying VMs" is section, but is not bolded or expandable.
  • "Applications Guide" is a section, but is not expandable or bolded in the sidenav.
  • When I go to a subpage in "Applications Guide" the sidenav collapses.
  • Accessing any subpage in the "Ray Cluster Management API" causes the sidenav to collapse. Also, it's a section, but is not bolded and subitems don't appear in the dropdown.

@peytondmurray
Copy link
Contributor Author

peytondmurray commented Nov 7, 2023

  • Make all blues the same

@peytondmurray
Copy link
Contributor Author

peytondmurray commented Nov 11, 2023

@emmyscode

On the "Ray Clusters Overview" page, the "Learn Key Concepts" button/card link jumps you down past the title on the "Key Concepts" page.

I'm seeing this on https://docs.ray.io/en/master as well. This is not due to any change I've made here, but the fact that the Learn Key Concepts button links to an anchor that is placed underneath the heading of the page. Really it should be linking to the doc, not an arbitrary sphinx ref. This is a pattern found in many places, but I think it would be best to clean this stuff up after the update is complete.

On the "Key Concepts" page, each heading (i.e. Ray Cluster, Head Node, Worker Node, Autoscaling, and Ray Jobs) when clicked shoots you up to the Ray Cluster heading.

I'm also seeing this on https://docs.ray.io/en/master; leaving for future cleanup.

When I'm on a page, it's not highlighted in the sidenav, which causes me to kind of lose where I am, especially in a deeply nested section like "Ray Clusters."
When I go to any subpage of "API References" under "Deploying VMs," the sidenav collapses completely and I lose my spot.
"API References" under "Deploying VMs" is section, but is not bolded or expandable.
"Applications Guide" is a section, but is not expandable or bolded in the sidenav.
When I go to a subpage in "Applications Guide" the sidenav collapses.
Accessing any subpage in the "Ray Cluster Management API" causes the sidenav to collapse. Also, it's a section, but is not bolded and subitems don't appear in the dropdown.

The rest of these are due to the fact that we don't currently have every single page accessible from the sidebar - it's sort of an arbitrary collection of links at the moment. @angelinalg and I met earlier this week and decided to put every page in the sidebar until we can find a better solution, so for now that's what I'll do. I'll notify once it's ready for you to look at.

@peytondmurray peytondmurray force-pushed the docs-deps-upgrade branch 5 times, most recently from 5d24e7b to b09e6a3 Compare November 14, 2023 00:42
@peytondmurray
Copy link
Contributor Author

I think I've addressed all the feedback from the docs team. I'll split this PR into reviewable chunks against a new branch, then we'll merge that branch to master at which point I'll close this PR.

@peytondmurray peytondmurray force-pushed the docs-deps-upgrade branch 2 times, most recently from 7b790bc to c93830b Compare November 14, 2023 03:54
Disable most recent `master` changes to CSS
Insert svg logo for ray train; add official ray blue color
Remove sphinx-tabs, it breaks per-page css/js and PST has tabs already
Fix tip nesting inside tab-item
Ray train logo is in place
Fix "Edit on GitHub" buttons
Hide "Hide Search Matches" button
Add developer guides subsections
Fix css on example gallery
Finished example gallery
Add Ray Data user guides
Fix "more-frameworks" logo placement; improve code block dev docs
Set fixed width for autosummary tables
Add back in CSAT widget and styles
Remove special tooling to make ecosystem grid
Fix ray-libraries card layout _without_ js
Remove js which builds top nav; fix assistant styles
Assistant widget now blurs background correctly
Remove _toc.yml; start adding missing sidebar links via toctree
In the midst of fixing the sidebar...
Well, this definitely works to add links to the sidebar.

    But the problem is that each worker is inserting into a separate copy of
    the sidebar Nav, so the links that are available in your sidebar depend
    on which worker was building the particular page you're looking at.

Switch to using toctree in sidebar
Implemented toctree as sidenav
Clean up styles; add back in marked.js, hl.js
Remove versionwarning extension; it's broken and we aren't using it
Add call to highlight code in splash.js
Include google analytics via PST; update hljs; fix ray intro video size;
use ray blue for various styles
Finish final changes to styling before rebase
@peytondmurray
Copy link
Contributor Author

Closing now that #41115 is merged.

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.

[docs] [docs infra] Finish Sphinx migration
7 participants