search

GeoTIFF / georaster

Wrapper around Georeferenced Rasters like GeoTIFF and soon JPG and PNG that provides a standard interface
Apache License 2.0
81 stars 32 forks source link

bilinear resample method is hard coded and not always a sensible option #56

Closed scazz010 closed 3 years ago

scazz010 commented 3 years ago

Describe the bug

Hiya,

https://github.com/GeoTIFF/georaster/blob/83d66c93fd6675b3340f6dca07e11c9c574d7ed1/src/index.js#L31

Bilinear is usually a pretty good option for things like DTMs. But it doesn't work at all for things like land cover (e.g. where 10 means forest and 12 means water, it never makes sense to combine these to an 11, which may mean desert)

Expected behavior Since the options object is passed directly to that function, an extra option that defaults to 'bilinear' is probably the most sensible way to solve this? According to the geotif docs 'nearest' is the only other (default) option. So, a different approach might be to follow the geotif library and make 'nearest' the default. Whilst not backward compatible, it might make understanding the library easier for new users. Nearest is also faster, so potentially a more sensible default for that reason too.

I'm about to fork and change this, so if you would like a PR, I can provide that

We had a chat a year or so ago, and I'm still using this library ;) Thanks again for the hard work! S

scazz010 commented 3 years ago

Also, as a side note. The underlying geotif library doesn't seem to understand nodata when it does the resampling? (Not your issue, but bloody annoying! I'm surprised that hasn't come up before)

https://github.com/geotiffjs/geotiff.js/blob/758455a14c900dd90a5a718110898229c3e86fa2/src/resample.js#L50

I'll open an issue in this library too. Seems kind of strange since a lot of no-data values I've seen as -9999

DanielJDufour commented 3 years ago

Hi, @scazz010 . Thank you for the excellent suggestion. A pull-request would be much appreciated!

DanielJDufour commented 3 years ago

Could we still default to bilinear, but allow someone to pass in the resampleMethod if they wanted to change it to nearest?

DanielJDufour commented 3 years ago

Hi, @scazz010 . I merged the PR and republish as georaster version 1.4.0! I'll close this issue now, but please open another issue for any other issues. Thank you again!