Skip to content

[Datasets] Add missing passthrough args to read_images() - #32942

Merged
clarkzinzow merged 4 commits into
masterfrom
datasets/feat/read-images-passthroughs
Mar 29, 2023
Merged

[Datasets] Add missing passthrough args to read_images()#32942
clarkzinzow merged 4 commits into
masterfrom
datasets/feat/read-images-passthroughs

Conversation

@clarkzinzow

Copy link
Copy Markdown
Contributor

This PR adds missing passthrough args to read_images(), such as ray_remote_args, arrow_open_file_args, and other misc. args popped in the base FileBasedDatasource such as compression. This PR also adds a **read_args catch-all to ImageDatasource._read_file(), which should add support for using the local:// protocol.

Related issue number

Closes #32941

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

@krfricke krfricke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@c21 c21 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LG assuming unit test to be added.

@clarkzinzow
clarkzinzow force-pushed the datasets/feat/read-images-passthroughs branch from d4ae194 to 701125f Compare March 29, 2023 17:22
Comment on lines +633 to +635
ray_remote_args: kwargs passed to ray.remote in the read tasks.
arrow_open_file_args: kwargs passed to
``pyarrow.fs.FileSystem.open_input_file``.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: can we follow the same ordering, to add these two parameters after meta_provider?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah yep, looks like these got reordered when resolving merge conflicts, I'll fix that!

@c21

c21 commented Mar 29, 2023

Copy link
Copy Markdown
Contributor

@zhe-thoughts - can you approve this?

@zhe-thoughts zhe-thoughts left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved for merging to master

@clarkzinzow
clarkzinzow merged commit 047abf5 into master Mar 29, 2023
@clarkzinzow
clarkzinzow deleted the datasets/feat/read-images-passthroughs branch March 29, 2023 22:31
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
…#32942)

This PR adds missing passthrough args to read_images(), such as ray_remote_args, arrow_open_file_args, and other misc. args popped in the base FileBasedDatasource such as compression. This PR also adds a **read_args catch-all to ImageDatasource._read_file(), which should add support for using the local:// protocol.

Signed-off-by: elliottower <elliot@elliottower.com>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
…#32942)

This PR adds missing passthrough args to read_images(), such as ray_remote_args, arrow_open_file_args, and other misc. args popped in the base FileBasedDatasource such as compression. This PR also adds a **read_args catch-all to ImageDatasource._read_file(), which should add support for using the local:// protocol.

Signed-off-by: Jack He <jackhe2345@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Datasets] Imagery reading doesn't work with local:// URIs

5 participants