fix rasterio chunking with s3 datasets

[rabernat]

• Closes rasterio chunks argument causes loading from s3 to fail #1816 (remove if there is no corresponding issue, which should only be the case for minor changes)
• Tests added (for all bug fixes or enhancements)
• Tests passed (for all non-documentation changes)
• Passes git diff upstream/master **/*py | flake8 --diff (remove if you did not edit any Python files)
• 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)

This is a simple fix for token generation of non-filename targets for rasterio.

The problem is that I have no idea how to test it without actually hitting s3 (which requires boto and aws credentials).

[shoyer]

You can test this by mocking os.path.getmtime, e.g.,

from unittest import mock  # but note that we use a special import for mock in xarray

with mock.patch('os.path.getmtime', side_effect=RuntimeError):
    os.path.getmtime('fooasdf.txt')

[shoyer]

and -> an

[shoyer]

Python 2 only has the super-class OSError, so that should probably be used instead here.

[rabernat]

I also refactored the rasterio test suite to eliminate a lot of boilerplate dataset creation and added tests for network urls. I'm not a geotiff expert, so please let me know if I have done anything silly.

[fmaussion]

Thanks @rabernat for the long overdue refactoring, althought this will mess with #1712 and #1740 quite a bit. Shouldv'e updated my PR earlier, d'oh!

[fmaussion]

I forgot about the @network decorator, this can be quite useful for other tests as well...

[rabernat]

this will mess with #1712 and #1740 quite a bit

I probably should have paid more attention to those PR's. My apologies.

Here is how I ended up refactoring the tests. I had to write a new test to catch the bug I discovered. I looked at the existing tests for guidance and saw lots of boilerplate. So rather than copying the boilerplate, I refactored it.

Let me know how I can help resolve this conflicts this may cause with the other open PRs.

[fmaussion]

#1740 is OK since the tests are still missing. My PR is a bit more problematic, but I can handle it. I would favor pushing yours forward now and I'll rebase my PR on top of it.

[shoyer]

Looks great, thank you!

[shoyer]

@rabernat this now has some conflicts to resolve. Can you fix those and merge this? Thanks!

[rabernat]

I resolved conflicts. LGTM before it gets stale again!

Is it bad form to merge my own PR?

[fmaussion]

test_nodata and test_nodata_missing are tests which were added in #1740, they shouldn't be deleted by your PR. It's fine if you don't refactorize them (I'll need to have a look at all the tests anyway once all conflicting PRs are merged, see #1843)

[shoyer]

@rabernat @fmaussion it's fine to merge your own PR once it's been reviewed & approved by another core developer.

[shoyer]

This looks good so let's get it in!

[rabernat]

@fmaussion - so sorry I accidentally deleted your tests. I was just rushing to resolve the conflicts so as not to hold up the other PRs. I hope this does not cause too much inconvenience.

[fmaussion]

@rabernat no problem, will go back the rasterio tests soon