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

[docs] fix build #34265

Merged
merged 9 commits into from
Apr 12, 2023
Merged

[docs] fix build #34265

merged 9 commits into from
Apr 12, 2023

Conversation

maxpumperla
Copy link
Contributor

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

@maxpumperla
[docs] fix build
a9570e2 
Signed-off-by: Max Pumperla <[email protected]>
@maxpumperla
fix doctests
0dd1824 
Signed-off-by: Max Pumperla <[email protected]>
@maxpumperla
last test
4865111 
Signed-off-by: Max Pumperla <[email protected]>
@maxpumperla
lint
fcb7dff 
Signed-off-by: Max Pumperla <[email protected]>
maxpumperla
maxpumperla commented Apr 11, 2023
View reviewed changes
doc/source/data/api/dataset.rst Outdated
@@ -149,3 +149,17 @@ Serialization
Dataset.has_serializable_lineage
Dataset.serialize_lineage
Dataset.deserialize_lineage


Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need these to be explicitly in a TOC somewhere. We can argue about the exact position after fixing the build.

angelinalg
angelinalg approved these changes Apr 11, 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

kouroshHakha
kouroshHakha approved these changes Apr 11, 2023
View reviewed changes
Copy link
Contributor

@kouroshHakha kouroshHakha left a comment

Choose a reason for hiding this comment

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

Thanks @maxpumperla. The build method got axed and got replaced with the setup method due to conflict with internal tf.keras.Model API. If you can replace that it will be great. Thanks.

ericl
ericl reviewed Apr 11, 2023
View reviewed changes
doc/source/data/api/dataset.rst Outdated
Dataset.fully_executed
Dataset.is_fully_executed
Dataset.lazy
Dataset.write_webdataset
Copy link
Contributor

Choose a reason for hiding this comment

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

This one should be added to "I/O and Conversion" list in dataset.rst

doc/source/data/api/dataset.rst Outdated
---------

.. autosummary::
:toctree: doc/
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh cool, this deprecates most of my fix PR here: #34228

Btw, is there a way to hide this internals section by default? Some of these are legacy backwards compatibility aliases that we don't want to expose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ericl yes, toctrees can be :hidden:, if this is what we want.

python/ray/data/_internal/execution/interfaces.py Outdated
@@ -191,22 +191,6 @@ class ExecutionOptions:
"""Common options for execution.

Some options may not be supported on all executors (e.g., resource limits).

Copy link
Contributor

Choose a reason for hiding this comment

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

I was able to fix this via adding the nosummary template: https://github.com/ray-project/ray/pull/34228/files#diff-b3a1d55836e0b2035d287e569a716fa02296221b15eee9babd6c2ea0f62a2ec7R13

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool, thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

@pcmoritz you can undo this particular diff now (fixed in master).

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, will do!

@maxpumperla @kouroshHakha
Update doc/source/rllib/package_ref/rl_modules.rst
7bcd081 
Co-authored-by: kourosh hakhamaneshi <[email protected]>
Signed-off-by: Max Pumperla <[email protected]>
@pcmoritz
Copy link
Contributor

^ I tried to fix the merge conflicts -- let's see if it passes and if yes we can merge it :)

cc @maxpumperla

@pcmoritz
Copy link
Contributor

Merging this now -- there is one more documentation failure that will be fixed by the RL team. Thanks for fixing all this @maxpumperla :)

@pcmoritz pcmoritz merged commit 0c56183 into master Apr 12, 2023
@pcmoritz pcmoritz deleted the mp_build_fixes branch April 12, 2023 23:41
@maxpumperla
Copy link
Contributor Author

@pcmoritz thank you for jumping in and resolving the conflicts!

vitsai pushed a commit to vitsai/ray that referenced this pull request Apr 17, 2023
@maxpumperla @kouroshHakha
@pcmoritz @vitsai
[docs] fix build (ray-project#34265)
3976502 
* [docs] fix build

Signed-off-by: Max Pumperla <[email protected]>

* fix doctests

Signed-off-by: Max Pumperla <[email protected]>

* last test

Signed-off-by: Max Pumperla <[email protected]>

* lint

Signed-off-by: Max Pumperla <[email protected]>

* Update doc/source/rllib/package_ref/rl_modules.rst

Co-authored-by: kourosh hakhamaneshi <[email protected]>
Signed-off-by: Max Pumperla <[email protected]>

* fixes

* revert diff

* whitespace

---------

Signed-off-by: Max Pumperla <[email protected]>
Signed-off-by: Philipp Moritz <[email protected]>
Co-authored-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: Philipp Moritz <[email protected]>
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
@maxpumperla @kouroshHakha
@pcmoritz @elliottower
[docs] fix build (ray-project#34265)
3b66972 
* [docs] fix build

Signed-off-by: Max Pumperla <[email protected]>

* fix doctests

Signed-off-by: Max Pumperla <[email protected]>

* last test

Signed-off-by: Max Pumperla <[email protected]>

* lint

Signed-off-by: Max Pumperla <[email protected]>

* Update doc/source/rllib/package_ref/rl_modules.rst

Co-authored-by: kourosh hakhamaneshi <[email protected]>
Signed-off-by: Max Pumperla <[email protected]>

* fixes

* revert diff

* whitespace

---------

Signed-off-by: Max Pumperla <[email protected]>
Signed-off-by: Philipp Moritz <[email protected]>
Co-authored-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: Philipp Moritz <[email protected]>
Signed-off-by: elliottower <[email protected]>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
@maxpumperla @kouroshHakha
@pcmoritz @ProjectsByJackHe
[docs] fix build (ray-project#34265)
cbe7cfb 
* [docs] fix build

Signed-off-by: Max Pumperla <[email protected]>

* fix doctests

Signed-off-by: Max Pumperla <[email protected]>

* last test

Signed-off-by: Max Pumperla <[email protected]>

* lint

Signed-off-by: Max Pumperla <[email protected]>

* Update doc/source/rllib/package_ref/rl_modules.rst

Co-authored-by: kourosh hakhamaneshi <[email protected]>
Signed-off-by: Max Pumperla <[email protected]>

* fixes

* revert diff

* whitespace

---------

Signed-off-by: Max Pumperla <[email protected]>
Signed-off-by: Philipp Moritz <[email protected]>
Co-authored-by: kourosh hakhamaneshi <[email protected]>
Co-authored-by: Philipp Moritz <[email protected]>
Signed-off-by: Jack He <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pcmoritz pcmoritz pcmoritz left review comments

@ericl ericl ericl left review comments

@angelinalg angelinalg angelinalg approved these changes

@kouroshHakha kouroshHakha kouroshHakha approved these changes

@scv119 scv119 scv119 approved these changes

@sven1977 sven1977 Awaiting requested review from sven1977

@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

@krfricke krfricke Awaiting requested review from krfricke

@clarkzinzow clarkzinzow Awaiting requested review from clarkzinzow

@jjyao jjyao Awaiting requested review from jjyao

@jianoaix jianoaix Awaiting requested review from jianoaix

@c21 c21 Awaiting requested review from c21

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@maxpumperla @pcmoritz @ericl @scv119 @kouroshHakha @angelinalg