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

[Doc] Restructure core API docs #32236

Merged
merged 59 commits into from
Feb 14, 2023
Merged

[Doc] Restructure core API docs #32236

merged 59 commits into from
Feb 14, 2023

Conversation

jjyao
Copy link
Collaborator

@jjyao jjyao commented Feb 6, 2023

Signed-off-by: Jiajun Yao jeromeyjj@gmail.com

Why are these changes needed?

Similar to #31204, refactor the core api reference for better layout and view.

Related issue number

Closes #32079

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 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 :(

Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
This reverts commit 7aa8d6f.
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
{% if methods %}
.. rubric:: {{ _('Methods') }}

.. autosummary::
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will generate stub page for each class method and attribute which will make the doc build even longer but that's what other libraries (e.g. pandas, numpy) are doing.

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @maxpumperla FYI.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jjyao is there a compromise we could find here? this seems to more than double the build time now. I'm in touch with support from readthedocs.org right now to make longer builds happen, but for now we can't pull this off.

just asking if we can think of a trade-off and move towards a more extensive solution later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we can try to not autosummary class members

Copy link
Contributor

Choose a reason for hiding this comment

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

yep, let's try this for now and see how it impacts build times. this PR increases rtd builds from 900s to 2000+, which will always time out

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Seems that doesn't work: https://buildkite.com/ray-project/oss-ci-build-pr/builds/11774#01863776-7701-40d6-974c-b6fe51e9d705 since we have places that manually autosummary class members which creates duplications with autoclass.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, so be it. we just have to make sure we stay below 3000s total, once all other libs merge their changes. might be risky.

@@ -13,4 +13,4 @@ API References
../rllib/package_ref/index.rst
../workflows/package-ref.rst
../cluster/package-overview.rst
../ray-core/package-ref.rst
../ray-core/reference/index.rst
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Checked several libraries (pandas, spark, numpy, scipy), seems there api docs have the following structure:

reference/
    index.rst
    group1.rst
    group2.rst
    api|generated/class1.rst
    api|generated/class1.rst

I feel it's better than

api/
    api.rst
    group1.rst
    group2.rst
    doc/class1.rst
    doc/class1.rst

so I tried it out in this PR. Would like to get other's opinions on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

note that every folder structure change impacts backward compatibility. the URLs will change, so I think we should keep the current structure (i.e. api, not reference, but we can of course rename to index etc.)

Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
@jjyao
Copy link
Collaborator Author

jjyao commented Feb 8, 2023

WARNING: [autosummary] failed to import 'ray.experimental.state.common.JobState.entrypoint': no module named ray.experimental.state.common.JobState.entrypoint
--
  | WARNING: [autosummary] failed to import 'ray.experimental.state.common.JobState.status': no module named ray.experimental.state.common.JobState.status

This is a known issue with autosummary: sphinx-doc/sphinx#9884.

@maxpumperla can we ignore it for now?

@jjyao jjyao marked this pull request as ready for review February 8, 2023 19:27
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
@jjyao jjyao merged commit 99d00ad into ray-project:master Feb 14, 2023
@jjyao jjyao deleted the jjyao/api branch February 14, 2023 06:57
@clarkzinzow
Copy link
Contributor

@jjyao
Copy link
Collaborator Author

jjyao commented Feb 14, 2023

Yup, I'm aware of that and I have a follow-up PR to fix it. Please don't revert this :)

@jjyao jjyao removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Feb 14, 2023
jjyao added a commit that referenced this pull request Feb 14, 2023
Similar to #31204, refactor the core api reference for better layout and view.

Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
jjyao added a commit that referenced this pull request Feb 16, 2023
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Co-authored-by: Justin Yu <justinvyu@anyscale.com>
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
Similar to ray-project#31204, refactor the core api reference for better layout and view.

Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
Similar to ray-project#31204, refactor the core api reference for better layout and view.

Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Signed-off-by: elliottower <elliot@elliottower.com>
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.

[core] Restructure core API docs
7 participants