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

Docs mock compiled modules #39666

Merged

Conversation

peytondmurray
Copy link
Contributor

@peytondmurray peytondmurray commented Sep 14, 2023

Why are these changes needed?

This PR does two things:

  1. Compiled ray modules are mocked out using sphinx's autodoc_mock_modules. This means we no longer need to roll our own mocking system for the compiled ray modules, and removes some unsightly type annotations in certain places in the docs.
  2. Intersphinx mappings were added for a lot of ray's dependencies. For types that are documented with sphinx and hosted elsewhere, our type annotations will now be clickable links to the appropriate documentation for that type. See for example the Python type annotations below, which are links to the pages of the official Python docs for those types:

image

NOTE: The changes to memory_utils.py are needed because here we mock out ray._raylet, a compiled module. Previously memory_utils.py was doing arithmetic on objects that are now mocked out, which would raise an exception when building the docs. I therefore had to move this arithmetic inside the functions that actually need these quantities to get the docs to build. I checked everywhere in the code base, and I can't see any reason that these are being computed at module initialization time, but if someone else sees a problem with this approach I'd be open to other suggestions.

In testing this change, I also noticed some incorrect type annotations for external dependencies, so I've fixed those too so that intersphinx can link them correctly.

Related issue number

Partially fulfills #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 marked this pull request as ready for review September 15, 2023 19:22
@peytondmurray peytondmurray changed the title [WIP] Docs mock compiled modules Docs mock compiled modules Sep 15, 2023
Copy link
Contributor

@angelinalg angelinalg left a comment

Choose a reason for hiding this comment

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

@maxpumperla, please review. Thanks!

Copy link
Contributor

@maxpumperla maxpumperla left a comment

Choose a reason for hiding this comment

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

Perfect, thanks so much @peytondmurray!

Signed-off-by: pdmurray <[email protected]>
@peytondmurray peytondmurray mentioned this pull request Sep 20, 2023
16 tasks
Copy link
Member

@cadedaniel cadedaniel left a comment

Choose a reason for hiding this comment

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

core changes LGTM

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 changes LGTM

@peytondmurray
Copy link
Contributor Author

Looks like there's an issue on the server side for https://lightning.ai/docs/pytorch/stable/objects.inv. I'll push a dummy commit to retrigger CI.

Signed-off-by: pdmurray <[email protected]>
@bveeramani bveeramani merged commit 7cd8f0d into ray-project:master Sep 25, 2023
2 checks passed
@peytondmurray peytondmurray deleted the docs-mock-compiled-modules branch September 25, 2023 23:42
simonsays1980 pushed a commit to simonsays1980/ray that referenced this pull request Sep 26, 2023
This PR does two things:

- Compiled ray modules are mocked out using sphinx's autodoc_mock_modules. This means we no longer need to roll our own mocking system for the compiled ray modules, and removes some unsightly type annotations in certain places in the docs.
- Intersphinx mappings were added for a lot of ray's dependencies. For types that are documented with sphinx and hosted elsewhere, our type annotations will now be clickable links to the appropriate documentation for that type. See for example the Python type annotations below, which are links to the pages of the official Python docs for those types:

Signed-off-by: pdmurray <[email protected]>
Co-authored-by: Max Pumperla <[email protected]>
Co-authored-by: Balaji Veeramani <[email protected]>
vymao pushed a commit to vymao/ray that referenced this pull request Oct 11, 2023
This PR does two things:

- Compiled ray modules are mocked out using sphinx's autodoc_mock_modules. This means we no longer need to roll our own mocking system for the compiled ray modules, and removes some unsightly type annotations in certain places in the docs.
- Intersphinx mappings were added for a lot of ray's dependencies. For types that are documented with sphinx and hosted elsewhere, our type annotations will now be clickable links to the appropriate documentation for that type. See for example the Python type annotations below, which are links to the pages of the official Python docs for those types:

Signed-off-by: pdmurray <[email protected]>
Co-authored-by: Max Pumperla <[email protected]>
Co-authored-by: Balaji Veeramani <[email protected]>
Signed-off-by: Victor <[email protected]>
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.

5 participants