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

[serve] Clean up API reference and various docstrings #34711

Merged
merged 33 commits into from
May 5, 2023
Merged

[serve] Clean up API reference and various docstrings #34711

merged 33 commits into from
May 5, 2023

Conversation

edoakes
Copy link
Contributor

@edoakes edoakes commented Apr 24, 2023 •
edited
Loading

Why are these changes needed?

Cleans up the API reference page formatting, adds/updates docstrings where applicable (corrects typing + verbiage, adds inline examples).

Related issue number

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

@edoakes
WIP
b1e151e 
Signed-off-by: Edward Oakes <[email protected]>
@edoakes
fix
d82bf89 
Signed-off-by: Edward Oakes <[email protected]>
@edoakes
working
c214fd9 
Signed-off-by: Edward Oakes <[email protected]>
@edoakes
fix getattr
a37508d 
Signed-off-by: Edward Oakes <[email protected]>
@edoakes
fix msg
1d9d676 
Signed-off-by: Edward Oakes <[email protected]>
@edoakes
wut
23be0d2 
Signed-off-by: Edward Oakes <[email protected]>
@edoakes
Merge branch 'master' of https://github.com/ray-project/ray into app-…
2700450 
…class
@edoakes
fix import
82412b6 
Signed-off-by: Edward Oakes <[email protected]>
@edoakes
fix tests
cd15020 
Signed-off-by: Edward Oakes <[email protected]>
@edoakes
rm imp
ff292f8 
Signed-off-by: Edward Oakes <[email protected]>
@edoakes
Merge branch 'master' of https://github.com/ray-project/ray into app-…
0ed6efd 
…class
@edoakes
working
2f05639 
Signed-off-by: Edward Oakes <[email protected]>
@edoakes edoakes changed the title [WIP][serve] API reference cleanups [serve] Clean up API reference and various docstrings Apr 25, 2023
@edoakes edoakes self-assigned this May 2, 2023
zcin
zcin reviewed May 2, 2023
View reviewed changes
python/ray/serve/deployment.py Outdated Show resolved Hide resolved
zcin
zcin approved these changes May 2, 2023
View reviewed changes
Copy link
Contributor

@zcin zcin left a comment

Choose a reason for hiding this comment

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

LGTM!

@edoakes @zcin
Update python/ray/serve/deployment.py
dc2113d 
Co-authored-by: Cindy Zhang <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
@akshay-anyscale akshay-anyscale mentioned this pull request May 2, 2023
Closed
@edoakes
Copy link
Contributor Author

edoakes commented May 3, 2023

@ray-project/ray-docs can I get a review on this please

angelinalg
angelinalg reviewed May 5, 2023
View reviewed changes
python/ray/serve/api.py Outdated Show resolved Hide resolved
angelinalg
angelinalg reviewed May 5, 2023
View reviewed changes
python/ray/serve/api.py Outdated Show resolved Hide resolved
angelinalg
angelinalg reviewed May 5, 2023
View reviewed changes
python/ray/serve/api.py Outdated Show resolved Hide resolved
angelinalg
angelinalg reviewed May 5, 2023
View reviewed changes
python/ray/serve/api.py Outdated Show resolved Hide resolved
angelinalg
angelinalg reviewed May 5, 2023
View reviewed changes
python/ray/serve/api.py Outdated Show resolved Hide resolved
angelinalg
angelinalg reviewed May 5, 2023
View reviewed changes
python/ray/serve/api.py Outdated Show resolved Hide resolved
angelinalg
angelinalg reviewed May 5, 2023
View reviewed changes
python/ray/serve/api.py Outdated Show resolved Hide resolved
angelinalg
angelinalg reviewed May 5, 2023
View reviewed changes
python/ray/serve/api.py Outdated Show resolved Hide resolved
angelinalg
angelinalg reviewed May 5, 2023
View reviewed changes
python/ray/serve/api.py Outdated Show resolved Hide resolved
angelinalg
angelinalg reviewed May 5, 2023
View reviewed changes
doc/source/serve/api/index.md

<!---
NOTE: `serve.deployment` and `serve.Deployment` have an autosummary-generated filename collision due to case insensitivity.
This is fixed by added custom filename mappings in `source/conf.py` (look for "autosummary_filename_map").
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This is fixed by added custom filename mappings in `source/conf.py` (look for "autosummary_filename_map").
This is fixed by added custom filename mappings in `source/conf.py`. (Look for "autosummary_filename_map".)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's actually correct as-is?

@edoakes @angelinalg
Update python/ray/serve/api.py
b05dd13 
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
richardliaw
richardliaw reviewed May 5, 2023
View reviewed changes
python/ray/serve/api.py
>>> # deployment_name#krcwoa
>>> serve.get_replica_context().replica_tag # doctest: +SKIP

.. code-block:: python
Copy link
Contributor

Choose a reason for hiding this comment

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

eventually we will move off this code-block thing, fyi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

onto what?

python/ray/serve/deployment.py Outdated Show resolved Hide resolved
@angelinalg
Copy link
Contributor

Had some minor copy edits. Please use the Add suggestion to batch button if you want to batch the suggestions into a single commit. Hope that helps.

angelinalg
angelinalg approved these changes May 5, 2023
View reviewed changes
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.

LGTM

@edoakes
Copy link
Contributor Author

edoakes commented May 5, 2023

Thanks @angelinalg !!

@edoakes
angelina comments
38eaa2f 
Signed-off-by: Edward Oakes <[email protected]>
@edoakes edoakes merged commit 13ba050 into ray-project:master May 5, 2023
architkulkarni pushed a commit to architkulkarni/ray that referenced this pull request May 16, 2023
@edoakes @zcin
@angelinalg @architkulkarni
[serve] Clean up API reference and various docstrings (ray-project#34711
b2e4e0d 
)

Cleans up the API reference page formatting, adds/updates docstrings where applicable (corrects typing + verbiage, adds inline examples).

Co-authored-by: Cindy Zhang <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardliaw richardliaw richardliaw approved these changes

@angelinalg angelinalg angelinalg approved these changes

@akshay-anyscale akshay-anyscale akshay-anyscale approved these changes

@zcin zcin zcin approved these changes

@shrekris-anyscale shrekris-anyscale Awaiting requested review from shrekris-anyscale

@sihanwang41 sihanwang41 Awaiting requested review from sihanwang41

@architkulkarni architkulkarni Awaiting requested review from architkulkarni

Assignees

@edoakes edoakes

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@edoakes @angelinalg @richardliaw @zcin @akshay-anyscale