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

Upgrade documentation techstack #6470

Merged

Conversation

saurabheights
Copy link
Contributor

@saurabheights saurabheights commented Nov 4, 2023

Type

  • Bug fix (non-breaking change which fixes an issue): Fixes #
  • New feature (non-breaking change which adds functionality). Resolves #
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

Focusing on upgrading documentation dependencies and removing dependency on open3d_sphinx_theme.

The changes done in open3d_sphinx_theme can be overridden without needing to fork another repository. This allows easier upgrade of documentation tech stack in future.

I only incorporated one change from
readthedocs/sphinx_rtd_theme@master...isl-org:open3d_sphinx_theme:master which is to support selection of different version of documentation.

@ssheorey Search is working.

WIP - This change breaks layout of references. Badly too. Almost fixed.

Checklist:

  • I have run python util/check_style.py --apply to apply Open3D code style
    to my code.
  • This PR changes Open3D behavior or adds new functionality.
    • Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
      updated accordingly.
    • I have added or updated C++ and / or Python unit tests OR included test
      results
      (e.g. screenshots or numbers) here.
  • I will follow up and update the code if CI fails.
  • For fork PRs, I have selected Allow edits from maintainers.

Description


This change is Reviewable

@saurabheights
Copy link
Contributor Author

Another Issue - There is a lot of extra vertical margin added between elements in docs.

@saurabheights saurabheights changed the title Khanduja/upgrade documentation techstack Upgrade documentation techstack Nov 4, 2023
@saurabheights
Copy link
Contributor Author

saurabheights commented Nov 4, 2023

image
JUST FYI - Replacing sphinx-rtd-theme with furo, which provides dark and light themes along with using system theme. Also, references works nicely.

References works well with both furo and sphinx-rtd-theme, only when docutils version <= 0.17.0.

pradyunsg/furo#729
readthedocs/sphinx_rtd_theme#1540

@ssheorey ssheorey self-requested a review November 5, 2023 00:50
@ssheorey
Copy link
Member

ssheorey commented Nov 5, 2023

Hi @saurabheights thanks for taking this up! The new theme looks more modern. Let me know when it's ready for review.

Copy link
Member

@ssheorey ssheorey left a comment

Choose a reason for hiding this comment

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

Looks great! Go ahead and switch to the furo theme if you like. Thanks!

@saurabheights
Copy link
Contributor Author

saurabheights commented Nov 10, 2023

@ssheorey Just an update on progress - furo theme works well until docutils version 0.17.0, so I upraded packages until this version. I have added minor css change for toctree captions. The references are slightly incorrect but we can ignore the issue for now as it is minor (I will try to find its fix as well). The main pending blocker is pradyunsg/furo#500 which provides support for multiple version. I am in process of overriding the theme to inject the html and css from http://www.open3d.org/docs/versions.js.

A solid upvote for using furo is the right navigation bar which shows TOC of webpage making navigating and search much easier for large classes. (Ignore bottom left, still working on navigation bar).

image

P.S. I am travelling international, so will be busy for next week. Please let me know if this PR is priority, say for next version release. I will try my best to wrap it up sooner.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -5,6 +5,7 @@ Open3D-ML

.. image:: https://raw.githubusercontent.com/isl-org/Open3D-ML/master/docs/images/getting_started_ml_visualizer.gif
:width: 480px
:align: center
Copy link
Contributor Author

@saurabheights saurabheights Nov 10, 2023

Choose a reason for hiding this comment

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

Wanted to highlight that since furo is center-aligned theme, unlike rtd, its better to go for center alignment for images. (IMO, although correct me if you have a contradicting thoughts).

@saurabheights
Copy link
Contributor Author

saurabheights commented Nov 10, 2023

@ssheorey Ready for review. Check each commit message to see summary. The reference page doesnt render well but I would prefer to wait for an upstream-fix. Lemme know if thats a blocker for merge.

Navigation panels are working well, although dont look great.

P.S. Side request - Please will it be possible to get some feedback on #6446. You can also reach out to me on discord.

@ssheorey ssheorey merged commit 2a11e0e into isl-org:master Nov 14, 2023
29 checks passed
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.

2 participants