Kwargs to rasterio open by pkopparla · Pull Request #5609 · pydata/xarray

pkopparla · 2021-07-15T14:21:02Z

ClosesAccessing COG overviews with read_rasterio #3269
User visible changes (including notable bug fixes) are documented in whats-new.rst

I've added a new kwargs argument to the function xarray.open_rasterio under xarray/xarray/backends/rasterio_.py which is passed on to the rasterio.open function as dicussed under issue #3269. I've added a line describing this change in whats-new.rst

I'm new to open source contributing, tried to follow instructions as much as I could. I am not sure how to write tests for this, would appreciate any help on that.

cheers,
Pushkar

pep8speaks · 2021-07-15T14:21:06Z

Hello @pkopparla! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-07-27 09:23:27 UTC

into kwargs-to-rasterio Updating to the latest xarray repo

github-actions · 2021-07-15T14:50:24Z

Unit Test Results

6 files ±0 6 suites ±0 48m 44s ⏱️ ±0s
16 200 tests ±0 14 475 ✔️ ±0 1 725 💤 ±0 0 ❌ ±0
90 396 runs ±0 82 231 ✔️ ±0 8 165 💤 ±0 0 ❌ ±0

Results for commit 78cec7c. ± Comparison against base commit 78cec7c.

♻️ This comment has been updated with latest results.

dcherian

Thanks @pkopparla This looks very nice.

I too am not sure what a test would look like> Perhaps @scottyhq or @snowman2 have ideas,

max-sixty · 2021-07-15T15:18:48Z

Should this pass **kwargs through rather than take an explicit kwargs arg?

Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>

pkopparla · 2021-07-15T15:39:04Z

Should this pass **kwargs through rather than take an explicit kwargs arg?

I am not sure about the nuances here, but the linked issue shows a h5netcdf function that follows the same pattern.

keewis · 2021-07-15T15:56:06Z

I would try to stay as close to open_dataset as possible, which would make migrating to rioxarray's engine easier once we deprecate open_rasterio. If I understand the signature of open_dataset correctly, this is called backend_kwargs?

dcherian · 2021-07-15T16:26:18Z

rioxarray.open_rasterio (and xr.open_zarr) uses the **kwargs format, so maybe that's the model to follow?

keewis · 2021-07-15T17:10:29Z

right, it seems it's both backend_kwargs and **kwargs for open_dataset.

snowman2 · 2021-07-15T18:18:43Z

For reference:

Co-authored-by: keewis <keewis@users.noreply.github.com>

pkopparla · 2021-07-16T07:19:24Z

Thanks everyone for the feedback. I've added a backend_kwargs and a **kwargs argument following the xarray.open_dataset style.

pkopparla · 2021-07-16T08:47:00Z

Looks like the backend_kwargs argument is breaking the function. Should I just get rid of it and leave in **kwargs?

keewis

Should I just get rid of it and leave in **kwargs?

probably. @aurghs, @alexamici, it would be good to hear your thoughts on this if you have the time.

Update to latest xarray repo

Updating to latest xarray repo

pkopparla · 2021-07-27T07:33:27Z

@dcherian @keewis Hey guys, a backend test is failing but I'm not sure why or how my change is affecting it. Would be grateful for a pointer.

keewis · 2021-07-27T07:40:24Z

this has been fixed / worked around on main, so the merge should have fixed it

pkopparla · 2021-07-27T08:09:27Z

@keewis thanks! Looks like most checks are successful now. Is there anything further I should do? Should I close the pull request?

keewis · 2021-07-27T08:24:34Z

no, this is fine, and the PR looks good to me. I'd say let's wait one more day to give more time for reviews and then merge by tomorrow.

aurghs · 2021-07-27T08:37:35Z

I would try to stay as close to open_dataset as possible, which would make migrating to rioxarray's engine easier once we deprecate open_rasterio. If I understand the signature of open_dataset correctly, this is called backend_kwargs?

The idea was to deprecate in the future open_datasetbackend_kwargs. Currently, in open_dataset signature, you can use either backend_kwargs or kwargs to pass the backend additional parameters. I would avoid adding backend_kwargs in open_rasterio interface.

keewis

makes sense. @pkopparla, could you remove backend_kwargs?

Co-authored-by: keewis <keewis@users.noreply.github.com>

pkopparla · 2021-07-27T09:24:10Z

@keewis Done!

keewis · 2021-07-27T10:02:21Z

thanks for the update, @pkopparla. I'll merge this tomorrow if there are no further comments.

keewis · 2021-07-30T07:09:10Z

Thanks, @pkopparla. I also noticed this is your first PR here. Welcome to xarray!

pkopparla · 2021-07-30T07:19:19Z

Thanks for leading me through the process @keewis , it's my first code contribution to any open-source project. Hope to do more in the future.

* main: (31 commits) Refactor index vs. coordinate variable(s) (pydata#5636) pre-commit: autoupdate hook versions (pydata#5685) Flexible Indexes: Avoid len(index) in map_blocks (pydata#5670) Speed up _mapping_repr (pydata#5661) update the link to `scipy`'s intersphinx file (pydata#5665) Bump styfle/cancel-workflow-action from 0.9.0 to 0.9.1 (pydata#5663) pre-commit: autoupdate hook versions (pydata#5660) fix the binder environment (pydata#5650) Update api.rst (pydata#5639) Kwargs to rasterio open (pydata#5609) Bump codecov/codecov-action from 1 to 2.0.2 (pydata#5633) new blank whats-new for v0.19.1 v0.19.0 release notes (pydata#5632) remove deprecations scheduled for 0.19 (pydata#5630) Make typing-extensions optional (pydata#5624) Plots get labels from pint arrays (pydata#5561) Add to_numpy() and as_numpy() methods (pydata#5568) pin fsspec (pydata#5627) pre-commit: autoupdate hook versions (pydata#5617) Add dataarray scatter with 3d support (pydata#4909) ...

* upstream/main: (31 commits) Refactor index vs. coordinate variable(s) (pydata#5636) pre-commit: autoupdate hook versions (pydata#5685) Flexible Indexes: Avoid len(index) in map_blocks (pydata#5670) Speed up _mapping_repr (pydata#5661) update the link to `scipy`'s intersphinx file (pydata#5665) Bump styfle/cancel-workflow-action from 0.9.0 to 0.9.1 (pydata#5663) pre-commit: autoupdate hook versions (pydata#5660) fix the binder environment (pydata#5650) Update api.rst (pydata#5639) Kwargs to rasterio open (pydata#5609) Bump codecov/codecov-action from 1 to 2.0.2 (pydata#5633) new blank whats-new for v0.19.1 v0.19.0 release notes (pydata#5632) remove deprecations scheduled for 0.19 (pydata#5630) Make typing-extensions optional (pydata#5624) Plots get labels from pint arrays (pydata#5561) Add to_numpy() and as_numpy() methods (pydata#5568) pin fsspec (pydata#5627) pre-commit: autoupdate hook versions (pydata#5617) Add dataarray scatter with 3d support (pydata#4909) ...

* upstream/main: (34 commits) Use same bool validator as other inputs (pydata#5703) conditionally disable bottleneck (pydata#5560) Refactor index vs. coordinate variable(s) (pydata#5636) pre-commit: autoupdate hook versions (pydata#5685) Flexible Indexes: Avoid len(index) in map_blocks (pydata#5670) Speed up _mapping_repr (pydata#5661) update the link to `scipy`'s intersphinx file (pydata#5665) Bump styfle/cancel-workflow-action from 0.9.0 to 0.9.1 (pydata#5663) pre-commit: autoupdate hook versions (pydata#5660) fix the binder environment (pydata#5650) Update api.rst (pydata#5639) Kwargs to rasterio open (pydata#5609) Bump codecov/codecov-action from 1 to 2.0.2 (pydata#5633) new blank whats-new for v0.19.1 v0.19.0 release notes (pydata#5632) remove deprecations scheduled for 0.19 (pydata#5630) Make typing-extensions optional (pydata#5624) Plots get labels from pint arrays (pydata#5561) Add to_numpy() and as_numpy() methods (pydata#5568) pin fsspec (pydata#5627) ...

* upstream/main: (307 commits) Use same bool validator as other inputs (pydata#5703) conditionally disable bottleneck (pydata#5560) Refactor index vs. coordinate variable(s) (pydata#5636) pre-commit: autoupdate hook versions (pydata#5685) Flexible Indexes: Avoid len(index) in map_blocks (pydata#5670) Speed up _mapping_repr (pydata#5661) update the link to `scipy`'s intersphinx file (pydata#5665) Bump styfle/cancel-workflow-action from 0.9.0 to 0.9.1 (pydata#5663) pre-commit: autoupdate hook versions (pydata#5660) fix the binder environment (pydata#5650) Update api.rst (pydata#5639) Kwargs to rasterio open (pydata#5609) Bump codecov/codecov-action from 1 to 2.0.2 (pydata#5633) new blank whats-new for v0.19.1 v0.19.0 release notes (pydata#5632) remove deprecations scheduled for 0.19 (pydata#5630) Make typing-extensions optional (pydata#5624) Plots get labels from pint arrays (pydata#5561) Add to_numpy() and as_numpy() methods (pydata#5568) pin fsspec (pydata#5627) ...

pkopparlaand others added 3 commits July 15, 2021 15:55

Added kwargs option to be passed to rasterio.open
2b429fb

Added description to whats-new.rst
1af777a

Merge branch 'pydata:main' into kwargs-to-rasterio
0ef8f2d

pkopparla added 2 commits July 15, 2021 16:23

Fixed whitespace
86afe8a

Merge branch 'kwargs-to-rasterio' ofhttps://github.com/pkopparla/xarray
01101e5
into kwargs-to-rasterio Updating to the latest xarray repo

dcherian reviewed Jul 15, 2021
View reviewed changes

Comment threaddoc/whats-new.rst Outdated

Update doc/whats-new.rst
39a45e7
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>

keewis reviewed Jul 15, 2021
View reviewed changes

Comment threaddoc/whats-new.rst Outdated

pkopparlaand others added 3 commits July 16, 2021 08:39

Update doc/whats-new.rst
768736c
Co-authored-by: keewis <keewis@users.noreply.github.com>

Updated whats-new.rst
d399b81

Merge branch 'pydata:main' into kwargs-to-rasterio
a82532d

keewis reviewed Jul 16, 2021
View reviewed changes

Comment threadxarray/backends/rasterio_.py Outdated

pkopparla added 4 commits July 16, 2021 16:04

Merged backend_kwargs with kwargs
08c74fc

Added check for no kwargs
f4ae878

Merge branch 'main' ofhttps://github.com/pydata/xarrayinto pydata-main
6d7f13c
Update to latest xarray repo

Merge branch 'pydata-main' into kwargs-to-rasterio
fd76cbb
Updating to latest xarray repo

keewis added the plan to merge Final call for comments label Jul 27, 2021

keewis reviewed Jul 27, 2021
View reviewed changes

Comment threaddoc/whats-new.rst Outdated

pkopparlaand others added 2 commits July 27, 2021 11:18

Update doc/whats-new.rst
bbda7ba
Co-authored-by: keewis <keewis@users.noreply.github.com>

Removed backend_kwargs
f0f0961

keewis requested review from jhamman and shoyer July 27, 2021 10:02

keewis merged commit 78cec7c into pydata:mainJul 30, 2021

Uh oh!

Uh oh!

Conversation

pkopparla commented Jul 15, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pep8speaks commented Jul 15, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2021-07-27 09:23:27 UTC

Uh oh!

github-actionsBot commented Jul 15, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Results

Uh oh!

dcherian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

max-sixty commented Jul 15, 2021

Uh oh!

pkopparla commented Jul 15, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keewis commented Jul 15, 2021

Uh oh!

dcherian commented Jul 15, 2021

Uh oh!

keewis commented Jul 15, 2021

Uh oh!

Uh oh!

snowman2 commented Jul 15, 2021

Uh oh!

pkopparla commented Jul 16, 2021

Uh oh!

pkopparla commented Jul 16, 2021

Uh oh!

keewis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pkopparla commented Jul 27, 2021

Uh oh!

keewis commented Jul 27, 2021

Uh oh!

pkopparla commented Jul 27, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keewis commented Jul 27, 2021

Uh oh!

aurghs commented Jul 27, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keewis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pkopparla commented Jul 27, 2021

Uh oh!

keewis commented Jul 27, 2021

Uh oh!

keewis commented Jul 30, 2021

Uh oh!

pkopparla commented Jul 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

pkopparla commented Jul 15, 2021•
edited
Loading

pep8speaks commented Jul 15, 2021•
edited
Loading

github-actionsBot commented Jul 15, 2021•
edited
Loading

pkopparla commented Jul 15, 2021•
edited
Loading

pkopparla commented Jul 27, 2021•
edited
Loading

aurghs commented Jul 27, 2021•
edited
Loading