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.

Sign up for GitHub

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

Jump to bottom

[Doc] Fix broken references in Ray Core documentation #45115

Merged
merged 2 commits into from
Jun 4, 2024
Merged

[Doc] Fix broken references in Ray Core documentation #45115

merged 2 commits into from
Jun 4, 2024

Conversation

peytondmurray
Copy link
Contributor

@peytondmurray peytondmurray commented May 2, 2024 •
edited
Loading

Why are these changes needed?

This PR fixes broken links in Ray Core documentation in preparation for turning on Sphinx's nitpicky mode.

Related issue number

Partially addresses #39658. Blocked until #45160 is merged.

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 force-pushed the docs-fix-core-bad-links-39658 branch from feab4b8 to d388810 Compare May 3, 2024 17:08
@peytondmurray peytondmurray mentioned this pull request May 9, 2024
Merged
8 tasks
@peytondmurray peytondmurray force-pushed the docs-fix-core-bad-links-39658 branch from 53b9d45 to d301a5e Compare May 13, 2024 23:08
@peytondmurray peytondmurray force-pushed the docs-fix-core-bad-links-39658 branch 5 times, most recently from 9ef7c2f to 7041316 Compare May 30, 2024 20:33
@peytondmurray peytondmurray changed the title [WIP] [Doc] Fix broken references in Ray Core documentation [Doc] Fix broken references in Ray Core documentation May 30, 2024
@peytondmurray peytondmurray marked this pull request as ready for review May 30, 2024 20:34
@peytondmurray peytondmurray requested review from rkooo567 and can-anyscale and removed request for rkooo567 May 30, 2024 20:34
can-anyscale
can-anyscale reviewed Jun 3, 2024
View reviewed changes
Copy link
Collaborator

@can-anyscale can-anyscale left a comment

Choose a reason for hiding this comment

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

thanks for the clean up, a bunch of dump questions mostly for me to learn

doc/source/ray-core/api/utility.rst Show resolved Hide resolved
doc/source/ray-more-libs/dask-on-ray.rst Outdated Show resolved Hide resolved
doc/source/ray-more-libs/dask-on-ray.rst Outdated Show resolved Hide resolved
python/ray/_private/worker.py Show resolved Hide resolved
@peytondmurray peytondmurray force-pushed the docs-fix-core-bad-links-39658 branch from 7041316 to 9cb0368 Compare June 3, 2024 21:45
can-anyscale
can-anyscale approved these changes Jun 3, 2024
View reviewed changes
Copy link
Collaborator

@can-anyscale can-anyscale left a comment

Choose a reason for hiding this comment

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

@peytondmurray peytondmurray force-pushed the docs-fix-core-bad-links-39658 branch from 9cb0368 to b7ff3cd Compare June 3, 2024 22:26
jjyao
jjyao requested changes Jun 3, 2024
View reviewed changes
python/ray/_private/worker.py Outdated
@@ -2708,14 +2708,14 @@ def put(
@PublicAPI
@client_mode_hook
def wait(
ray_waitables: List[Union["ObjectRef[R]", "ObjectRefGenerator[R]"]],
ray_waitables: Union[ObjectRef, ObjectRefGenerator],
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be a list.

Why changing this? Seems unrelated to broken referneces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, my mistake - I'll make this a list again.

Why changing this? Seems unrelated to broken referneces

sphinx.ext.autodoc uses type annotations to document parameters and return types for functions.

python/ray/util/state/api.py
@@ -1468,8 +1449,7 @@ def summarize_objects(
Dictionarified :class:`~ray.util.state.common.ObjectSummaries`

Raises:
Exceptions: :class:`RayStateApiException <ray.util.state.exception.RayStateApiException>` if the CLI
failed to query the data.
RayStateApiException: if the CLI failed to query the data.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this generate the link?

Copy link
Contributor Author

@peytondmurray peytondmurray Jun 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes. sphinx.ext.autodoc does this automatically if you have a valid docstring format - the previous format was not, so the link needed to be added manually.

jjyao
jjyao reviewed Jun 3, 2024
View reviewed changes
python/ray/_private/worker.py
*,
num_returns: int = 1,
timeout: Optional[float] = None,
fetch_local: bool = True,
) -> Tuple[
List[Union["ObjectRef[R]", "ObjectRefGenerator[R]"]],
List[Union["ObjectRef[R]", "ObjectRefGenerator[R]"]],
List[Union[ObjectRef, ObjectRefGenerator]],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is R removed so that we can generate link to ObjectRef?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, although if you'd prefer to keep R, we can import it at the top and instead have these be ObjectRef[R]. So depending on how you'd like to handle this, we can either keep it as is or add the TypeVar.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rkooo567 what do you want here since you added R I think

jjyao
jjyao approved these changes Jun 4, 2024
View reviewed changes
Copy link
Collaborator

@jjyao jjyao left a comment

Choose a reason for hiding this comment

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

Thanks for the fix.

@jjyao jjyao enabled auto-merge (squash) June 4, 2024 04:46
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label Jun 4, 2024
@jjyao jjyao merged commit 4f8eb2f into ray-project:master Jun 4, 2024
7 checks passed
@peytondmurray peytondmurray deleted the docs-fix-core-bad-links-39658 branch June 4, 2024 15:30
richardsliu pushed a commit to richardsliu/ray that referenced this pull request Jun 12, 2024
@peytondmurray @richardsliu
[Doc] Fix broken references in Ray Core documentation (ray-project#45115
16933df 
)

Signed-off-by: pdmurray <[email protected]>
Signed-off-by: Richard Liu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@can-anyscale can-anyscale can-anyscale approved these changes

@jjyao jjyao jjyao approved these changes

@hongpeng-guo hongpeng-guo Awaiting requested review from hongpeng-guo

@justinvyu justinvyu Awaiting requested review from justinvyu

@matthewdeng matthewdeng Awaiting requested review from matthewdeng

@raulchen raulchen Awaiting requested review from raulchen

@woshiyyya woshiyyya Awaiting requested review from woshiyyya

Assignees

@jjyao jjyao

Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@peytondmurray @jjyao @can-anyscale