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

ENH: Add storage_options to read_parquet #2107

Merged
merged 7 commits into from Sep 23, 2021
Merged

ENH: Add storage_options to read_parquet #2107

merged 7 commits into from Sep 23, 2021

Conversation

TomAugspurger
Copy link
Contributor

Closes #2071

@TomAugspurger
Copy link
Contributor Author

I believe the CI failure is unrelated.

@martinfleis
Copy link
Member

Yes, fixed in #2101

jorisvandenbossche
jorisvandenbossche reviewed Sep 13, 2021
View reviewed changes
Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR!

I have a slight preference for keeping to pass through string URIs for eg S3 if no storage options are provided, since pyarrow can handle those (the annoying part is to know what pyarrow supports, though, since this is only limited (currently s3 and hdfs). Similar to pandas-dev/pandas#41194. But can also leave this as a follow-up.

geopandas/io/tests/test_arrow.py Show resolved Hide resolved
geopandas/io/tests/test_arrow.py Outdated Show resolved Hide resolved
geopandas/io/arrow.py Outdated Show resolved Hide resolved
geopandas/io/arrow.py Outdated Show resolved Hide resolved
@TomAugspurger
Copy link
Contributor Author

TomAugspurger commented Sep 13, 2021 •
edited

I have a slight preference for keeping to pass through string URIs for eg S3 if no storage options are provided

I copied your implementation from the pandas PR, but didn't add any tests. Are we wanting a full moto-based S3 testing setup here?

Can I get another approval to run CI? :)

@martinfleis martinfleis added this to the 0.10 milestone Sep 19, 2021
@TomAugspurger
Copy link
Contributor Author

Hopefully my latest commit fixes the test coverage issue (could I get another CI run @jorisvandenbossche or @martinfleis?)

@jorisvandenbossche jorisvandenbossche changed the title Added storage_options to read_parquet ENH: Add storage_options to read_parquet Sep 23, 2021
jorisvandenbossche
jorisvandenbossche approved these changes Sep 23, 2021
View reviewed changes
Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

geopandas/io/tests/test_arrow.py Show resolved Hide resolved
@jorisvandenbossche jorisvandenbossche merged commit 32e7cf3 into geopandas:master Sep 23, 2021
@jorisvandenbossche
Copy link
Member

Thanks @TomAugspurger !

@TomAugspurger TomAugspurger deleted the feature/storage-options branch September 23, 2021 18:50
@knaaptime
Copy link

agreed, thanks all!

@TomAugspurger TomAugspurger mentioned this pull request Sep 24, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorisvandenbossche jorisvandenbossche jorisvandenbossche approved these changes

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

Successfully merging this pull request may close these issues.

ENH: support storage_options argument in read_parquet
4 participants
@TomAugspurger @martinfleis @jorisvandenbossche @knaaptime