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

[Data] [Docs] Standardize API Refs for Input/Output #37017

Merged
merged 21 commits into from
Jul 7, 2023
Merged

[Data] [Docs] Standardize API Refs for Input/Output #37017

merged 21 commits into from
Jul 7, 2023

Conversation

amogkam
Copy link
Contributor

@amogkam amogkam commented Jul 1, 2023

Standardize API refs for some of the I/O APIs.

Why are these changes needed?

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

@amogkam
wip
c7c74ce 
Signed-off-by: amogkam <[email protected]>
@amogkam
wip
e401bef 
Signed-off-by: amogkam <[email protected]>
@amogkam
Merge branch 'master' of github.com:ray-project/ray into standardize-…
e2ddefc 
…api-ref

Signed-off-by: amogkam <[email protected]>
@amogkam
wip
c47a761 
Signed-off-by: amogkam <[email protected]>
@amogkam
finish
0ab2df9 
Signed-off-by: amogkam <[email protected]>
@amogkam
fixes
7c2bb34 
Signed-off-by: amogkam <[email protected]>
@amogkam
fixes
e37e60e 
Signed-off-by: amogkam <[email protected]>
@amogkam
fixes
3d7bb19 
Signed-off-by: amogkam <[email protected]>
@amogkam
fix
05276c9 
Signed-off-by: amogkam <[email protected]>
bveeramani
bveeramani approved these changes Jul 5, 2023
View reviewed changes
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.

Nice! LGTM. Would just recommend moving comments out of REPL and into normal text to improve the readability of rendered examples.

python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Show resolved Hide resolved
python/ray/data/read_api.py Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/dataset.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
angelinalg
angelinalg reviewed Jul 5, 2023
View reviewed changes
python/ray/data/dataset.py Outdated Show resolved Hide resolved
python/ray/data/dataset.py Outdated Show resolved Hide resolved
python/ray/data/dataset.py Outdated Show resolved Hide resolved
python/ray/data/dataset.py Outdated Show resolved Hide resolved
python/ray/data/dataset.py Outdated Show resolved Hide resolved
python/ray/data/dataset.py Outdated Show resolved Hide resolved
python/ray/data/dataset.py Outdated Show resolved Hide resolved
python/ray/data/dataset.py Outdated Show resolved Hide resolved
python/ray/data/dataset.py Show resolved Hide resolved
python/ray/data/dataset.py Outdated Show resolved Hide resolved
angelinalg
angelinalg approved these changes Jul 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.

Lots of nits for polish. Hope it helps.

python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
python/ray/data/read_api.py Outdated Show resolved Hide resolved
angelinalg
angelinalg approved these changes Jul 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.

Lots of nits for polish. Hope it helps.

amogkam and others added 10 commits July 5, 2023 19:37
@amogkam
fixes
b262050 
Signed-off-by: amogkam <[email protected]>
@amogkam
more fixes
c4bd84c 
Signed-off-by: amogkam <[email protected]>
@amogkam
comments
655ea93 
Signed-off-by: amogkam <[email protected]>
@amogkam
comments
dc0ceac 
Signed-off-by: amogkam <[email protected]>
@amogkam @angelinalg
Apply suggestions from code review
895cb6c 
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: Amog Kamsetty <[email protected]>
@amogkam
Merge branch 'master' of github.com:ray-project/ray into standardize-…
40d6526 
…api-ref
@amogkam
fixes
b4a8bf0 
Signed-off-by: amogkam <[email protected]>
@amogkam
update
49d261a 
Signed-off-by: amogkam <[email protected]>
@amogkam
Merge branch 'standardize-api-ref' of github.com:amogkam/ray into sta…
7ed8c99 
…ndardize-api-ref

Signed-off-by: amogkam <[email protected]>
@amogkam
lint
487aa33 
Signed-off-by: amogkam <[email protected]>
sven1977
sven1977 approved these changes Jul 7, 2023
View reviewed changes
Copy link
Contributor

@sven1977 sven1977 left a comment

Choose a reason for hiding this comment

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

Looks good to me (from RLlib perspective). Thanks for the fix @amogkam !

@amogkam amogkam merged commit 957f9b7 into ray-project:master Jul 7, 2023
@amogkam amogkam deleted the standardize-api-ref branch July 7, 2023 16:32
arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023
@amogkam @angelinalg @arvind-chandra
[Data] [Docs] Standardize API Refs for Input/Output (ray-project#37017)
e845a0c 
Standardize API refs for some of the I/O APIs.

---------

Signed-off-by: amogkam <[email protected]>
Signed-off-by: Amog Kamsetty <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: e428265 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@angelinalg angelinalg angelinalg approved these changes

@bveeramani bveeramani bveeramani approved these changes

@sven1977 sven1977 sven1977 approved these changes

@ericl ericl Awaiting requested review from ericl

@scv119 scv119 Awaiting requested review from scv119

@c21 c21 Awaiting requested review from c21

@scottjlee scottjlee Awaiting requested review from scottjlee scottjlee is a code owner

@raulchen raulchen Awaiting requested review from raulchen raulchen is a code owner

@gjoliver gjoliver Awaiting requested review from gjoliver

@avnishn avnishn Awaiting requested review from avnishn

@ArturNiederfahrenhorst ArturNiederfahrenhorst Awaiting requested review from ArturNiederfahrenhorst

@smorad smorad Awaiting requested review from smorad

@maxpumperla maxpumperla Awaiting requested review from maxpumperla

@kouroshHakha kouroshHakha Awaiting requested review from kouroshHakha

@krfricke krfricke Awaiting requested review from krfricke

Assignees

@c21 c21

@bveeramani bveeramani

@angelinalg angelinalg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@amogkam @sven1977 @bveeramani @angelinalg @c21