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 links in Ray docs - turn on nitpicky mode #39658

Closed
peytondmurray opened this issue Sep 14, 2023 · 2 comments · Fixed by #46103
Closed

[Doc] Fix broken links in Ray docs - turn on nitpicky mode #39658

peytondmurray opened this issue Sep 14, 2023 · 2 comments · Fixed by #46103
Assignees
@peytondmurray
Labels
docs An issue or change related to documentation P1 Issue that should be fixed within a few weeks serve Ray Serve Related Issue

Comments

@peytondmurray
Copy link
Contributor

peytondmurray commented Sep 14, 2023 •
edited
Loading

Description

Running sphinx with nitpicky mode enabled generates warnings about all references that cannot be found. Currently we don't do this, and as a result we are missing broken sphinx references, like these the two references to private methods in this section.

I ran make develop with nitpicky mode enabled and count 5435 broken references in the documentation. Lots of these will probably be fixable with some tweaks to documentation build parameters, intersphinx mappings, and so on, but I'm starting this issue here to keep track of them. CC @angelinalg

Here's the full list: warnings.txt

Aside from fixing these reference, we should also aim to enable nitpicky mode in the sphinx docs so these don't keep sneaking past CI.
@peytondmurray peytondmurray added triage Needs triage (eg: priority, bug/not-bug, and owning component) docs An issue or change related to documentation labels Sep 14, 2023
@peytondmurray peytondmurray self-assigned this Sep 14, 2023
@matthewdeng
Copy link
Contributor

Nice! I tried this last week as well but did not follow up, thanks for creating an issue to track this. 😄

@anyscalesam anyscalesam added serve Ray Serve Related Issue P1 Issue that should be fixed within a few weeks and removed triage Needs triage (eg: priority, bug/not-bug, and owning component) labels Sep 26, 2023
@angelinalg angelinalg changed the title [Doc] Fix broken links in Ray docs [Doc] Fix broken links in Ray docs - turn on nitpicky mode Nov 10, 2023
@angelinalg angelinalg mentioned this issue Nov 10, 2023
Open
@peytondmurray
Copy link
Contributor Author

peytondmurray commented Apr 16, 2024 •
edited
Loading

Moving on this now. This will be accomplished in a few separate PRs to minimize codeowner impact.

  • Fix issues in each library separately
  • Turn on nitpicky mode; set default_role = 'code' instead of py:obj so that we stop interpreting everything in single backticks as a reference to a python object. This accounts for the majority of the issues because people use single backticks to mean "highlight this content as code" even though RST syntax uses double backticks for that. Additionally, add
nitpick_ignore_regex = [
    ('py:class', '.*')
]

to ignore the sphinx issues that arise from using user-defined types. This has been something brought up multiple times on StackOverflow and the Sphinx repo, so this has been a persistent issue for Sphinx in the past. There might be a way to get rid of this rule but for now this could be a good first step.

This was referenced Apr 24, 2024
Merged
Merged
This was referenced May 2, 2024
Merged
Merged
Merged
Merged
@peytondmurray peytondmurray mentioned this issue May 9, 2024
Merged
8 tasks
aslonnie pushed a commit that referenced this issue May 28, 2024
@peytondmurray
[Core] Add placeholder callback methods as members of RayDaskCallback (…
c4d6bcf 
…#45160)

## Why are these changes needed?

This PR adds the overridable methods for `RayDaskCallback` to the base
class.

Previously, the user had a few choices for instantiating a
`RayDaskCallback`:

```python
RayDaskCallback(ray_presubmit=my_custom_presubmit_func)
```

or

```python
with RayDaskCallback(ray_presubmit=my_custom_presubmit_func) as cb:
    ...
```

or by subclassing:

```python
class MyCallback(RayDaskCallback):

    def _ray_presubmit(self, task, key, deps):
        ....
```

Currently the constructor for `RayDaskCallback` dynamically generates
the callback methods and attaches them to the class instance upon
instantiation using `setattr`. This pattern doesn't work with developer
tooling including language servers, static code analysis tools, or
documentation generators because these functions are generated
dynamically. Additionally the documentation for dask-on-ray instructs
users to use the third approach (using subclassing) rather than passing
methods to the constructor. Finally, the docstring for
`RayDaskCallback.__init__` currently has function signatures and
pseudo-docstrings for the callback methods the user should be
implementing. This docstring is itself malformed, so Sphinx mistakenly
tries to interpret parts of this as references which do not exist.

At the request of @c21 I've actually implemented these functions as they
appear in the docstring. Type hints are incomplete here, but match what
was already in the previous docstring. I also added a summary in the
dask-on-ray documentation.

Unblocks #39658.
This was referenced Jun 17, 2024
Merged
Merged
can-anyscale pushed a commit that referenced this issue Jun 20, 2024
@peytondmurray
[Doc] Clean up remaining RLlib broken links (#46102)
49b57c5 
## Why are these changes needed?

This PR cleans up the last remaining broken links in the RLlib
documentation.

## Related issue number

Partially completes #39658.

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [x] I've run `scripts/format.sh` to lint the changes in this PR.
- [x] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [x] 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.
- [x] 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
   - [x] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: pdmurray <peynmurray@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@peytondmurray peytondmurray

Labels
docs An issue or change related to documentation P1 Issue that should be fixed within a few weeks serve Ray Serve Related Issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Doc] Turn on nitpicky mode peytondmurray/ray
3 participants
@matthewdeng @peytondmurray @anyscalesam