Various issues with new Canvas.raster

[philippjfr]

Currently the newly Canvas.raster method for regridding makes several assumption that keep it from being general. Here are the issues I encountered working with it:

• It assumes a 3D array shape even though the regridding happens in 2D. Ideally it would check whether the data is a 2D or 3D xarray.DataArray and work correctly on both. The datashader.util.calc_res function needs to be updated.
• The calc_res utility also currently assumes that the coordinate arrays are named x and y, which is not a safe assumption to make in general.
• The attrs inherited from the input array should be optional i.e. this line in Canvas.raster should be conditional in some way: attrs = dict(res=res[0], nodata=source._file_obj.nodata)
• If the ycoords are ascending rather than descending the yres ends up negative, resulting in a wrong bounding box. Need to consider flipped coordinates along both axes.
• When the canvas x_range and y_range are smaller than the canvas range, the data is not sliced correctly, i.e. even though some of the data is situated outside of the canvas all the data is still included.

To visualize the last issue here's what happens when you try to regrid an image with a bounding box of (-0.5, -0.5, 0.5, 0.5) onto a canvas with a bounding box of (-0.2, -0.2, 0.2, 0.2) this is the result:

i.e. the whole image has been squeezed into the new canvas rather than appropriately excluding samples that fall outside the new bounding box. @gbrener @jbednar

[jbednar]

Looks like merging that PR was premature. @gbrener, will you be able to address these issues on Monday? If not, I guess we should back out those changes for now.

[gbrener]

@philippjfr @jbednar Did the previous implementation of cvs.raster() meet these requirements? Either way it would be very helpful if we had unit tests for these cases (decorated with @pytest.mark.skip even though they'd be failing). I'd be happy to add that to my TODO list if @philippjfr has too much on his plate.

[philippjfr]

Did the previous implementation of cvs.raster() meet these requirements?

No probably not since rasterio datasets are probably quite well defined and consistent, issues 1-4 would probably not have occurred in that case. I'd have to check on the final issue to see if that was handled correctly before, that one is probably the issue that's most difficult to fix.

[gbrener]

@philippjfr The last bullet should now be addressed in PR #389. Please let me know if you have any issues.

[gbrener]

With PR #389 now merged, I am marking this issue closed.