search

locationtech / geotrellis

GeoTrellis is a geographic data processing engine for high performance applications.
http://geotrellis.io
Other
1.33k stars 360 forks source link

Consider a better crop function declaration for all GeoTrellis raster types #2687

Open pomadchin opened 6 years ago

pomadchin commented 6 years ago

Current crop function defined for all Raster types returns a T, instead of returning an Option[T] type. It causes semantic and logical issues with understanding how crop function should behave in case of a crop call by a not intersecting area. PR https://github.com/locationtech/geotrellis/pull/2684 would standardize crop function exceptions and behaviour, but probably it's not the thing finally we want to achieve.

def crop(...): Option[T]

It's unclear do we want to introduce it in terms of 2.0 or not. @echeipesh

pomadchin commented 6 years ago

Also probably it would make some sense to have a cropOption method in addition to an unsafe crop, instead of reimplementing all current crop methods.

moradology commented 6 years ago

Seems unlikely to make it into 2.0 this late in the game, but I am partial to defaulting to Option results if we revisit the API. Yeah, that will force the addition of .get calls (likely inside a try/catch block). But that's a good change if throwing is what you want your program to do - it makes such sites just a little more visible and a little more obvious and it doesn't require us to clutter users' heads with renamed methods that do basically the same thing