Optimizing raster.icrop

[vschaffn]

Resolves #659

Description

• Changes order of icrop bounding box to match with Rasterio.BoundingBox.
• Adds a parameter distance_unit: Literal["georeferenced", "pixel"] = "georeferenced" in georeferencing._crop to match with the use of icrop.

Tests

• Updates crop tests.

Doc

• Updates crop section in raster_class.md.

[adebardo]

Are we sure that the DEM is not completely load before crop ?

[rhugonnet]

Perfect, thanks!

[vschaffn]

@rhugonnet @adebardo thanks for your feedback !
I have corrected the Raster.icrop method :

• If the raster is not loaded, I use _load_rio with a window to just load the cropped raster
• If the raster is already loaded I kept the same way to do
• I replaced the transform translation by Raster.translate

I also fixed a sign error in _translate, and added some more tests

The loading check can't be set up in the tests because the raster is always loaded after icrop

[rhugonnet]

Thanks!

I'm not sure I understand the logic of the part of the PR regarding removing the dependency to crop()? To me that's a huge benefit. It allows to have a fully consistent behaviour, with lots of tests already written in crop(), without having to duplicate the same code (what is done here in the end, by reading with rio.Windows).
And it doesn't affect performance: converting 4 indices to 4 coordinates is instantaneous.

[vschaffn]

@adebardo @rhugonnet @adehecq I have changed the implementation of the icrop as discussed on 28/03/2025. Does this work for you? 😃

[rhugonnet]

Perfect for me! Great idea to make it work through a distance_unit argument for consistency with other functions 😉

[adebardo]

Thanks for the work @vschaffn ! :) and @rhugonnet for the review :) I merge