-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
Conversation
f8acef4
to
9d4af67
Compare
28f4429
to
9ffb661
Compare
doc/Makefile
Outdated
@@ -2,7 +2,7 @@ | |||
# | |||
|
|||
# You can set these variables from the command line. | |||
SPHINXOPTS = -a -E | |||
SPHINXOPTS = -W |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
da598cb
to
3a15242
Compare
Few comments from the design side:
|
@simran-2797 Thanks for taking a look. I'll implement the color changes you made in the mockups.
Yes. The tooltip for the widget explains that the switch can be Re: example gallery - this is something I still need to finish up. Thanks for the reminder! |
|
That's because you're hovering over an image that was inserted as a .. 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. |
|
|
There was a problem hiding this 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?

- Nit: This is more of a personal preference, but I think the code text is more readable without the grey background.

- 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.

- 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

@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! |
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 |
3409696
to
9eec386
Compare
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: 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? |
Review of "Ray Clusters" sectionI'm only pointing our discrepancies between the current state and the upgrade, not necessarily sharing an opinion about how the side navigation should work.
|
|
4f6ae8d
to
8cc77ea
Compare
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
I'm also seeing this on https://docs.ray.io/en/master; leaving for future cleanup.
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. |
5d24e7b
to
b09e6a3
Compare
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 |
7b790bc
to
c93830b
Compare
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
c93830b
to
2ad478d
Compare
Closing now that #41115 is merged. |
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:
margin
directives; they are only implemented insphinx-book-theme
, which we are no longer using. These have all been converted tonote
directives.sphinx-external-toc
has been removed. There are a lot of orphaned pages; I've added these to the appropriate pages.LandingPageBG.jpg
: the docs landing page intended to use this but has accidentally been targetingLandingPageBG.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.cluster/kubernetes/troubleshooting/troubleshooting.html#gpu-multitenancy
, there is nogpu-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 developmentfail_on_warning
in.readthedocs.yml
Related issue number
Closes #37944.
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.