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][doc] Add DatasetConfig -> DataConfig migration guide #37278

Merged
merged 10 commits into from
Jul 13, 2023
Merged

[data][doc] Add DatasetConfig -> DataConfig migration guide #37278

merged 10 commits into from
Jul 13, 2023

Conversation

raulchen
Copy link
Contributor

@raulchen raulchen commented Jul 11, 2023 •
edited
Loading

Why are these changes needed?

  • Add DatasetConfig -> DataConfig migration guide
  • Move ray.train.data_config to ray.train._internal.data_config

Related issue number

closes #35958

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

@raulchen
migration guide
7e1ed3e 
Signed-off-by: Hao Chen <chenh1024@gmail.com>
raulchen added 3 commits July 10, 2023 17:33
@raulchen
blank line
bf5fe47 
Signed-off-by: Hao Chen <chenh1024@gmail.com>
@raulchen
link
dc5caf8 
Signed-off-by: Hao Chen <chenh1024@gmail.com>
@raulchen
fix
f1a6b93 
Signed-off-by: Hao Chen <chenh1024@gmail.com>
pcmoritz
pcmoritz reviewed Jul 11, 2023
View reviewed changes
doc/source/ray-air/doc_code/air_ingest_migration.py Outdated
# __legacy_api_end__

# __new_api__
from ray.train.data_config import DataConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be clear here that ray.train.data_config is not a public API. All the configs should be included via ray.train directly going forward. Also see the PR description of #36706 and the REP ray-project/enhancements#36 :)

Copy link
Contributor

Choose a reason for hiding this comment

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

@ericl maybe we should rename data_config to _data_config so there are not two ways to do this and people get confused?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should put it in a _internal folder then probably, to follow convention of other libraries.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, let's replace ray.train.data_config with ray.train everywhere in this PR before merging, and then we can do the _internal refactor later as time permits (doesn't need to be part of 2.6).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I assume nothing is needed in this PR, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is -- you need to replace ray.train.data_config.DataConfig with ray.train.DataConfig everywhere :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, I didn't know it's already in ray.train. I think there may be other places using ray.train.data_config as well. Will update all.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, that's awesome :)

raulchen added 2 commits July 11, 2023 15:22
@raulchen
package
20ee0a5 
Signed-off-by: Hao Chen <chenh1024@gmail.com>
@raulchen
revert doc
c0601ad 
Signed-off-by: Hao Chen <chenh1024@gmail.com>
pcmoritz
pcmoritz approved these changes Jul 12, 2023
View reviewed changes
Copy link
Contributor

@pcmoritz pcmoritz left a comment

Choose a reason for hiding this comment

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

Nice, feel free to merge if the CI passes :)

raulchen added 4 commits July 12, 2023 09:14
@raulchen
fix
36c6d66 
Signed-off-by: Hao Chen <chenh1024@gmail.com>
@raulchen
move to _internal
67f0d87 
Signed-off-by: Hao Chen <chenh1024@gmail.com>
@raulchen
doc
1892f5f 
Signed-off-by: Hao Chen <chenh1024@gmail.com>
@raulchen
fix
247b980 
Signed-off-by: Hao Chen <chenh1024@gmail.com>
@raulchen
Copy link
Contributor Author

@pcmoritz also moved data_config to _internal. Does this PR need to be cherry-picked to 2.6.0?

@pcmoritz
Copy link
Contributor

Thanks, that's great! Yes, it needs to be cherry-picked :)

@raulchen raulchen merged commit e233498 into ray-project:master Jul 13, 2023
@raulchen raulchen deleted the dataconfig-migration-doc branch July 13, 2023 01:02
raulchen added a commit to raulchen/ray that referenced this pull request Jul 13, 2023
@raulchen
[data][doc] Add DatasetConfig -> DataConfig migration guide (ray-proj…
0688169 
…ect#37278)

- Add DatasetConfig -> DataConfig migration guide
- Move ray.train.data_config to ray.train._internal.data_config

---------

Signed-off-by: Hao Chen <chenh1024@gmail.com>
@raulchen raulchen mentioned this pull request Jul 13, 2023
Merged
8 tasks
@raulchen raulchen mentioned this pull request Jul 13, 2023
Merged
8 tasks
bveeramani pushed a commit that referenced this pull request Jul 13, 2023
@raulchen
[cherry-pick][data][doc] Add DatasetConfig -> DataConfig migration gu…
abd287f 
…ide (#37383)

* [data][doc] Add DatasetConfig -> DataConfig migration guide  (#37278)

- Add DatasetConfig -> DataConfig migration guide
- Move ray.train.data_config to ray.train._internal.data_config

---------

Signed-off-by: Hao Chen <chenh1024@gmail.com>

* fix

Signed-off-by: Hao Chen <chenh1024@gmail.com>

---------

Signed-off-by: Hao Chen <chenh1024@gmail.com>
pcmoritz pushed a commit that referenced this pull request Jul 13, 2023
@raulchen
[minor] Fix DataConfig package in API doc (#37386)
49e884d 
##37278 moved `ray.train.data_config` to `ray.train._internal.data_config`. But missed this place.
Bhav00 pushed a commit to Bhav00/ray that referenced this pull request Jul 28, 2023
@raulchen @Bhav00
[data][doc] Add DatasetConfig -> DataConfig migration guide (ray-proj…
9d65e46 
…ect#37278)

- Add DatasetConfig -> DataConfig migration guide
- Move ray.train.data_config to ray.train._internal.data_config

---------

Signed-off-by: Hao Chen <chenh1024@gmail.com>
NripeshN pushed a commit to NripeshN/ray that referenced this pull request Aug 15, 2023
@raulchen @NripeshN
[data][doc] Add DatasetConfig -> DataConfig migration guide (ray-proj…
1485425 
…ect#37278)

- Add DatasetConfig -> DataConfig migration guide
- Move ray.train.data_config to ray.train._internal.data_config

---------

Signed-off-by: Hao Chen <chenh1024@gmail.com>
Signed-off-by: NripeshN <nn2012@hw.ac.uk>
NripeshN pushed a commit to NripeshN/ray that referenced this pull request Aug 15, 2023
@raulchen @NripeshN
[minor] Fix DataConfig package in API doc (ray-project#37386)
1082fed 
#ray-project#37278 moved `ray.train.data_config` to `ray.train._internal.data_config`. But missed this place.

Signed-off-by: NripeshN <nn2012@hw.ac.uk>
arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023
@raulchen @arvind-chandra
[data][doc] Add DatasetConfig -> DataConfig migration guide (ray-proj…
da77f0f 
…ect#37278)

- Add DatasetConfig -> DataConfig migration guide
- Move ray.train.data_config to ray.train._internal.data_config

---------

Signed-off-by: Hao Chen <chenh1024@gmail.com>
Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023
@raulchen @arvind-chandra
[minor] Fix DataConfig package in API doc (ray-project#37386)
e5c2079 
#ray-project#37278 moved `ray.train.data_config` to `ray.train._internal.data_config`. But missed this place.

Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
vymao pushed a commit to vymao/ray that referenced this pull request Oct 11, 2023
@raulchen
[data][doc] Add DatasetConfig -> DataConfig migration guide (ray-proj…
90b0ac8 
…ect#37278)

- Add DatasetConfig -> DataConfig migration guide
- Move ray.train.data_config to ray.train._internal.data_config

---------

Signed-off-by: Hao Chen <chenh1024@gmail.com>
Signed-off-by: Victor <vctr.y.m@example.com>
vymao pushed a commit to vymao/ray that referenced this pull request Oct 11, 2023
@raulchen
[minor] Fix DataConfig package in API doc (ray-project#37386)
e813d31 
#ray-project#37278 moved `ray.train.data_config` to `ray.train._internal.data_config`. But missed this place.

Signed-off-by: Victor <vctr.y.m@example.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericl ericl ericl approved these changes

@pcmoritz pcmoritz pcmoritz approved these changes

@richardliaw richardliaw Awaiting requested review from richardliaw

@gjoliver gjoliver Awaiting requested review from gjoliver

@krfricke krfricke Awaiting requested review from krfricke

@xwjiang2010 xwjiang2010 Awaiting requested review from xwjiang2010

@amogkam amogkam Awaiting requested review from amogkam

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

@Yard1 Yard1 Awaiting requested review from Yard1

@maxpumperla maxpumperla Awaiting requested review from maxpumperla

Assignees

@ericl ericl

@pcmoritz pcmoritz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[data] [docs] Update docs to reflect new Data+Train API
3 participants
@raulchen @pcmoritz @ericl