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

[RLlib] Updating codeblocks in RLlib part 1 #37271

Merged
merged 21 commits into from
Oct 24, 2023
Merged

[RLlib] Updating codeblocks in RLlib part 1 #37271

merged 21 commits into from
Oct 24, 2023

Conversation

avnishn
Copy link
Contributor

@avnishn avnishn commented Jul 10, 2023

This pr attempts to do 3 things:

  1. remove instances of codeblock python in rllib source
  2. remove any instances of doctest: +skip in rllib source
  3. remove any >>> style blocks of code in favor of testcode.

Many of the code examples will remain untested, as they are going to be deprecated over the next few releases.

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

@avnishn
[RLlib] Remove instances of doctest skip where possible and replace c…
d5205ba 
…ode-block with testcode

Signed-off-by: Avnish <avnishnarayan@gmail.com>
@avnishn
Add more test code
b1bfaa1 
Signed-off-by: Avnish <avnishnarayan@gmail.com>
@avnishn
Fix code doc testing in more files
8005fa6 
Signed-off-by: Avnish <avnishnarayan@gmail.com>
@avnishn
Adding files that have instances of >>>
2aaffce 
Signed-off-by: Avnish <avnishnarayan@gmail.com>
@avnishn
Remove instances of codeblock python in rllib src
edab938 
Signed-off-by: Avnish <avnishnarayan@gmail.com>
@avnishn
Lint
65ba74c 
Signed-off-by: Avnish <avnishnarayan@gmail.com>
avnishn and others added 9 commits July 10, 2023 18:28
@avnishn
Fix doc typo
c10ea63 
Signed-off-by: Avnish <avnishnarayan@gmail.com>
@bveeramani
Update BUILD
0d15ee5 
Signed-off-by: Balaji Veeramani <balaji@anyscale.com>
@bveeramani
Merge branch 'master' into pr/37271
e952990 
Signed-off-by: Balaji Veeramani <balaji@anyscale.com>
@bveeramani
Update BUILD
da87726 
Signed-off-by: Balaji Veeramani <balaji@anyscale.com>
@bveeramani
Update BUILD
b9adcca 
Signed-off-by: Balaji Veeramani <balaji@anyscale.com>
@bveeramani
Update BUILD
8ba2637 
Signed-off-by: Balaji Veeramani <balaji@anyscale.com>
@bveeramani
Merge branch 'master' into pr/37271
a137313 
Signed-off-by: Balaji Veeramani <balaji@anyscale.com>
@bveeramani
Skip test that timesout
273f2f1 
Signed-off-by: Balaji Veeramani <balaji@anyscale.com>
@bveeramani
Add newlines
1b66c67 
Signed-off-by: Balaji Veeramani <balaji@anyscale.com>
bveeramani
bveeramani approved these changes Oct 17, 2023
View reviewed changes
rllib/algorithms/algorithm_config.py Show resolved Hide resolved
rllib/algorithms/dreamerv3/dreamerv3.py Outdated Show resolved Hide resolved
rllib/algorithms/ppo/ppo.py Outdated Show resolved Hide resolved
@bveeramani bveeramani assigned sven1977 and unassigned kouroshHakha Oct 17, 2023
sven1977
sven1977 approved these changes Oct 19, 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.

LGTM. Thanks for this huge effort @avnishn!

@bveeramani
Update dreamerv3.py
79f5862 
Signed-off-by: Balaji Veeramani <balaji@anyscale.com>
angelinalg
angelinalg approved these changes Oct 24, 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.

Deferring to the engineers to review the code.

@angelinalg
Copy link
Contributor

Thank you for this big effort, @avnishn !!

@bveeramani bveeramani merged commit 887eddd into ray-project:master Oct 24, 2023
32 of 33 checks passed
@rickyyx rickyyx mentioned this pull request Dec 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sven1977 sven1977 sven1977 approved these changes

@bveeramani bveeramani bveeramani approved these changes

@angelinalg angelinalg angelinalg approved these changes

@gjoliver gjoliver Awaiting requested review from gjoliver

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

@smorad smorad Awaiting requested review from smorad

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

@kouroshHakha kouroshHakha Awaiting requested review from kouroshHakha

@krfricke krfricke Awaiting requested review from krfricke

Assignees

@sven1977 sven1977

@bveeramani bveeramani

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@avnishn @angelinalg @sven1977 @bveeramani @kouroshHakha