cogeotiff / rio-cogeo

Cloud Optimized GeoTIFF creation and validation plugin for rasterio
https://cogeotiff.github.io/rio-cogeo/
BSD 3-Clause "New" or "Revised" License
308 stars 42 forks source link

Clarify arguments used only when producing "web-optimized" COGs #277

Closed alexismanin closed 7 months ago

alexismanin commented 7 months ago

Hi,

It looks like some arguments to cogeo.cog_translate function are only applied if web_optimized argument is set to True, like tms, zoom_level, etc.

I failed to see them as "specialized" arguments on first look, and I had to look at the function source code for a better understanding.

Therefore, I see two possible improvements:

If any of the above suggestion looks good to you, I offer to submit a PR for it.

vincentsarago commented 7 months ago

👋 Bonjour @alexismanin

Thanks for starting this discussion.

Yes, I'm sorry about this. I think what happened is that when I started working on the WebOptimization part, there was only one option web_optimized and then when we enabled more options, we tried to follow GDAL COG options.

I think we could do:

alexismanin commented 7 months ago

Just a side-note: resampling does not seem bound to web-optimization, because it is used by rasterio warp (WarpedVRT), and that is done anyway.

If I understand well, no reprojection is configurable except through the tms argument, but the warp may still make sense if user has specified dtype parameter. Although I am wondering if a warp operator that only change output dtype will perform any interpolation/resample.

In any case, I will give a try to your suggestion.

vincentsarago commented 7 months ago

@alexismanin, the VRT is used also when

but yeah the resampling option shouldn't have any effect when not using the web-optimization. I think I though at one point of adding the dst-crs option but this will conflict with the tms option so I preferred not to pursue.

Although I am wondering if a warp operator that only change output dtype will perform any interpolation/resample.

I hope not 😅 , I can also see that we didn't add tests for the dtype option

vincentsarago commented 7 months ago

I think we are good to close this 🙏 @alexismanin