Watershed delineation: handling the case where flow_dir doesn't have an SRS.

[phargogh]

Also noting this change in HISTORY and testing.

Fixes #254

[emlys]

@phargogh this looks good, but I'm not sure I understand when it would be OK to have a missing SRS? How do we know that the flow direction raster intersects with the other inputs?

[phargogh]

when it would be OK to have a missing SRS? How do we know that the flow direction raster intersects with the other inputs?

Well, we don't know for sure! But if the units of the geotransform and geometries are all the same, it may be reasonable to assume that the user knows what they are doing and continue running. So we would be assuming that the inputs are in some projected coordinate system and valid for delineating watersheds. I don't think that's a reasonable assumption to make in InVEST, specifically, but I think it could be OK for a geoprocessing routine like this.

What do you think?

[emlys]

I wonder if it makes sense to have a larger conversation about this and decide when/if this makes sense in general, so we can be consistent within pygeoprocessing? For instance, maybe it's also ok for the outflow vector to have no SRS? But if both inputs have no SRS, is that a problem?

[phargogh]

The consistency question is a good one, and probably worth a discussion! I'll add it to the agenda for an upcoming software team call.

In the shorter term, it turns out that we have a valid use case for where there might not be a defined spatial reference: in the various supported raster formats that don't support georeferencing. Taking this CEOS example file from the GDAL tests, the SRS's WKT is falsey:

>>> from osgeo import gdal
>>> ds = gdal.Open('/Users/jdouglass/Downloads/IMAGERY-75K.L-3')
>>> ds.GetProjection()
''

[phargogh]

Thanks for your thoughts and the review @emlys ! I created a new issue #341 for the general pygeoprocessing question in line with the discussion this morning, and I improved the docstring in the watershed delineation routine.

[emlys]

Thanks for adding that note to the docstring! It just looks like some extra carraige returns got in there, otherwise good!

[phargogh]

Huh, I'm so used to my editor chomping them that I missed them in the diff! Thanks @emlys I think they should be clear now

[emlys]

Sorry @phargogh but I'm seeing one more ^M on line 631, those line ending characters are sneaky!

[phargogh]

Ugh, the lesson for me here is to be very careful when copying code out of a git diff! Thanks @emlys and I'm sorry for missing these!

Patched and pushed in 5ab4a46