-
Notifications
You must be signed in to change notification settings - Fork 7.2k
[WIP] [Docs] Docs deps upgrade #39766
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
Conversation
f8acef4 to
9d4af67
Compare
28f4429 to
9ffb661
Compare
doc/Makefile
Outdated
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 -Emakes sphinx rebuild the docs each time instead of using the existing build artifacts-Wmakes 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:
margindirectives; they are only implemented insphinx-book-theme, which we are no longer using. These have all been converted tonotedirectives.sphinx-external-tochas 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-multitenancyref. 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
Makefilechanges to speed up developmentfail_on_warningin.readthedocs.ymlRelated issue number
Closes #37944.
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.