added some logic to deal with rasterio objects in addition to filepaths #2589

scottyhq · 2018-12-04T05:13:33Z

…h strings

Closes Enabling rasterio.vrt.WarpedVRT with xr.open_rasterio #2588
Tests added (for all bug fixes or enhancements)
Fully documented, including whats-new.rst for all changes and api.rst for new API (remove if this change should not be visible to users, e.g., if it is an internal clean-up, or if this is part of a larger project that will be documented later)

…h strings

pep8speaks · 2018-12-04T05:13:37Z

Hello @scottyhq! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on December 23, 2018 at 18:24 Hours UTC

scottyhq · 2018-12-04T05:14:26Z

Following up on dask/dask#3255 @mrocklin, @shoyer, @jhamman

fmaussion

Thanks! I only have a few comments (I don't use these things myself), but the change seems harmless enough to me (once the tests pass ;).

Is there a way to test the functionality without actually going online?

And can you also add an entry to whatsnew.rst?

fmaussion · 2018-12-04T12:24:44Z

xarray/backends/rasterio_.py

@@ -2,7 +2,8 @@
 import warnings
 from collections import OrderedDict
 from distutils.version import LooseVersion
-
+import rasterio
+from rasterio.vrt import WarpedVRT


Imports of optional dependencies can unfortunately not happen at the module top level - import them only when needed in the function scope

fmaussion · 2018-12-04T12:26:32Z

xarray/tests/test_backends.py

+                    with xr.open_rasterio(tmp_file) as actual:
+                        assert_allclose(actual, expected)
+
+    def test_rasterio_vrt(self):


For network tests we have a special decorator (@network), see https://github.com/pydata/xarray/blob/55f21deff4c2b42bd6ead4dbe26a1b123337913a/xarray/tests/test_tutorial.py (although that's the only use of it as it seems?)

scottyhq · 2018-12-18T23:57:08Z

thanks for the feedback @fmaussion, I think I've addressed your suggestions, let me know if anything else needs adjusting

shoyer · 2018-12-19T01:24:01Z

xarray/backends/rasterio_.py

@@ -123,6 +124,42 @@ def __getitem__(self, key):
            key, self.shape, indexing.IndexingSupport.OUTER, self._getitem)


+class RasterioVRTWrapper(RasterioArrayWrapper):


Rather than adding a subclass with a lot of duplicated logic, could you add this into the base RasterioArrayWrapper class?

Something like:

def __init__(self, manager, vrt_params=None): ... riods = riods if vrt_params is None else WarpedVRT(riods, **vrt_params)

Good point, just moved the changes to RasterioArrayWrapper

shoyer · 2018-12-19T03:56:27Z

xarray/tests/test_backends.py

+    def test_rasterio_vrt_network(self):
+        import rasterio
+
+        url = 'https://storage.googleapis.com/\


I didn't even know this form of line-wrapping for strings was possible in Python :)

shoyer · 2018-12-19T17:18:53Z

These tests are failing on our "py36-rasterio-0.36" build, e.g.,

    def test_rasterio_vrt(self):
        import rasterio
        # tmp_file default crs is UTM: CRS({'init': 'epsg:32618'}
        with create_tmp_geotiff() as (tmp_file, expected):
            with rasterio.open(tmp_file) as src:
>               with rasterio.vrt.WarpedVRT(src, crs='epsg:4326') as vrt:
E               AttributeError: module 'rasterio' has no attribute 'vrt'

https://travis-ci.org/pydata/xarray/builds/469845498?utm_source=github_status&utm_medium=notification

Should we increase the minimum required version of rasterio or guard these imports?

scottyhq · 2018-12-19T17:49:19Z

I think the minimum rasterio version should be increased to 1.0. For whatever reason the conda defaults channel hasn't been updated since 0.36 (Jun 14, 2016!). There are many important changes in 1.0 and beyond, and those releases are available via both pip and conda-forge.

…into open-rasterio-vrt

jhamman · 2018-12-19T17:52:25Z

I'd be personally fine bumping our minimum version of rasterio to 1.0. There have been multiple minor releases since then so I would suggest the 1.0 version has stabilized by now.

shoyer · 2018-12-19T18:26:25Z

OK, please update our installation page at docs/installing.rst, too. This should also definitely be mentioned in "what's new"

mrocklin · 2018-12-19T18:34:47Z

For whatever reason the conda defaults channel hasn't been updated since 0.36 (Jun 14, 2016!).

@jjhelmus is there a good way to report things like this other than pinging you directly? (which I'm more than happy to continue doing :))

jjhelmus · 2018-12-19T21:14:18Z

@jjhelmus is there a good way to report things like this other than pinging you directly?

Opening and issue in the anaconda-issues repository is the best option at this time for requesting a package update.

I'm looking at updating the rasterio package in defaults this week. Something should be available by the end of the week.

mrocklin · 2018-12-19T21:17:52Z

ContinuumIO/anaconda-issues#10443

…

On Wed, Dec 19, 2018 at 4:14 PM Jonathan J. Helmus ***@***.***> wrote: @jjhelmus <https://github.com/jjhelmus> is there a good way to report things like this other than pinging you directly? Opening and issue in the anaconda-issues <https://github.com/ContinuumIO/anaconda-issues> repository is the best option at this time for requesting a package update. I'm looking at updating the rasterio package in defaults this week. Something should be available by the end of the week. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2589 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASszMr1wxsC7gjEdyH3dfrDU5kig_LBks5u6qwugaJpZM4Y_8DQ> .

jjhelmus · 2018-12-21T23:30:14Z

rasterio 1.0.13 packages are now available in defaults for all supported platforms and python version.

mrocklin · 2018-12-22T00:38:34Z

For whatever reason the conda defaults channel hasn't been updated since 0.36 (Jun 14, 2016!).

It looks like @jjhelmus resolved this upstream . It seems like https://github.com/ContinuumIO/anaconda-issues is a good issue tracker to know :)

mrocklin · 2018-12-22T00:43:16Z

mrocklin@carbon:~$ conda search rasterio=1
Loading channels: done
# Name                  Version           Build  Channel             
rasterio                 1.0.13  py27hc38cc03_0  pkgs/main           
rasterio                 1.0.13  py36hc38cc03_0  pkgs/main           
rasterio                 1.0.13  py37hc38cc03_0  pkgs/main

* master: DEP: drop python 2 support and associated ci mods (pydata#2637) TST: silence warnings from bottleneck (pydata#2638) revert to dev version DOC: fix docstrings and doc build for 0.11.1 Source encoding always set when opening datasets (pydata#2626) Add flake check to travis (pydata#2632) Fix dayofweek and dayofyear attributes from dates generated by cftime_range (pydata#2633) silence import warning (pydata#2635) fill_value in shift (pydata#2470) Flake fixed (pydata#2629) Allow passing of positional arguments in `apply` for Groupby objects (pydata#2413) Fix failure in time encoding for pandas < 0.21.1 (pydata#2630) Fix multiindex selection (pydata#2621) Close files when CachingFileManager is garbage collected (pydata#2595) added some logic to deal with rasterio objects in addition to filepaths (pydata#2589) Get 0d slices of ndarrays directly from indexing (pydata#2625) FIX Don't raise a deprecation warning for xarray.ufuncs.{angle,iscomplex} (pydata#2615) CF: also decode time bounds when available (pydata#2571)

shoyer · 2019-07-05T23:13:49Z

test_rasterio_vrt_network is failing in continuous integration tests, now: #3083

added some logic to deal with rasterio objects in addition to filepat…

35c5afb

…h strings

fmaussion reviewed Dec 4, 2018

View reviewed changes

added no network test, pep8 compliance, whatsnew.rst

35f9947

shoyer reviewed Dec 19, 2018

View reviewed changes

scottyhq and others added 2 commits December 18, 2018 19:46

removed subclass, added to base RasterioArrayWrapper

ce56d35

Merge branch 'master' into open-rasterio-vrt

cd6fcb9

shoyer reviewed Dec 19, 2018

View reviewed changes

upped rasterio test version to > 1

a113b50

Merge branch 'open-rasterio-vrt' of https://github.com/scottyhq/xarray …

de21485

…into open-rasterio-vrt

specified rasterio version should be greater than 1

dde9428

Merge branch 'master' into open-rasterio-vrt

ea068fe

shoyer merged commit 9352b3c into pydata:master Dec 23, 2018

dcherian mentioned this pull request Jan 8, 2019

rasterio released v1 as stable #2316

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added some logic to deal with rasterio objects in addition to filepaths #2589

added some logic to deal with rasterio objects in addition to filepaths #2589

scottyhq commented Dec 4, 2018 •

edited by shoyer

pep8speaks commented Dec 4, 2018 •

edited

scottyhq commented Dec 4, 2018

fmaussion left a comment

fmaussion Dec 4, 2018

fmaussion Dec 4, 2018

scottyhq commented Dec 18, 2018

shoyer Dec 19, 2018

scottyhq Dec 19, 2018 •

edited

shoyer Dec 19, 2018

shoyer commented Dec 19, 2018 •

edited

scottyhq commented Dec 19, 2018

jhamman commented Dec 19, 2018

shoyer commented Dec 19, 2018

mrocklin commented Dec 19, 2018

jjhelmus commented Dec 19, 2018

mrocklin commented Dec 19, 2018 via email

jjhelmus commented Dec 21, 2018

mrocklin commented Dec 22, 2018

mrocklin commented Dec 22, 2018

shoyer commented Jul 5, 2019

		@@ -123,6 +124,42 @@ def __getitem__(self, key):
		key, self.shape, indexing.IndexingSupport.OUTER, self._getitem)


		class RasterioVRTWrapper(RasterioArrayWrapper):

added some logic to deal with rasterio objects in addition to filepaths #2589

added some logic to deal with rasterio objects in addition to filepaths #2589

Conversation

scottyhq commented Dec 4, 2018 • edited by shoyer

pep8speaks commented Dec 4, 2018 • edited

Comment last updated on December 23, 2018 at 18:24 Hours UTC

scottyhq commented Dec 4, 2018

fmaussion left a comment

Choose a reason for hiding this comment

fmaussion Dec 4, 2018

Choose a reason for hiding this comment

fmaussion Dec 4, 2018

Choose a reason for hiding this comment

scottyhq commented Dec 18, 2018

shoyer Dec 19, 2018

Choose a reason for hiding this comment

scottyhq Dec 19, 2018 • edited

Choose a reason for hiding this comment

shoyer Dec 19, 2018

Choose a reason for hiding this comment

shoyer commented Dec 19, 2018 • edited

scottyhq commented Dec 19, 2018

jhamman commented Dec 19, 2018

shoyer commented Dec 19, 2018

mrocklin commented Dec 19, 2018

jjhelmus commented Dec 19, 2018

mrocklin commented Dec 19, 2018 via email

jjhelmus commented Dec 21, 2018

mrocklin commented Dec 22, 2018

mrocklin commented Dec 22, 2018

shoyer commented Jul 5, 2019

scottyhq commented Dec 4, 2018 •

edited by shoyer

pep8speaks commented Dec 4, 2018 •

edited

scottyhq Dec 19, 2018 •

edited

shoyer commented Dec 19, 2018 •

edited